Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 6/9] dt-bindings: Document rk3399 Gru/Kevin
From: Heiko Stuebner @ 2016-12-07 19:15 UTC (permalink / raw)
  To: Brian Norris
  Cc: linux-rockchip, linux-kernel, Caesar Wang, Doug Anderson,
	devicetree, Rob Herring, Stephen Barber, linux-arm-kernel,
	Chris Zhong
In-Reply-To: <20161207174139.GA87970@google.com>

Am Mittwoch, 7. Dezember 2016, 09:41:39 CET schrieb Brian Norris:
> On Wed, Dec 07, 2016 at 06:12:13PM +0100, Heiko Stuebner wrote:
> > Hi Brian,
> > 
> > Am Donnerstag, 1. Dezember 2016, 18:27:30 CET schrieb Brian Norris:
> > > Gru is a base dev board for a family of devices, including Kevin. Both
> > > utilize Rockchip RK3399, and they share much of their design.
> > > 
> > > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > > ---
> > > 
> > >  Documentation/devicetree/bindings/arm/rockchip.txt | 20
> > > 
> > > ++++++++++++++++++++ 1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt
> > > b/Documentation/devicetree/bindings/arm/rockchip.txt index
> > > cc4ace6397ab..830e13f5890c 100644
> > > --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> > > +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
> > > @@ -99,6 +99,26 @@ Rockchip platforms device tree bindings
> > > 
> > >  		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
> > >  		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
> > > 
> > > +- Google Gru (dev-board):
> > boards sorted alphabetically please
> > 
> > Brian, Gru, Jaq, ... Kevin, ...
> > 
> > While the sorting of old boards is not right yet, new boards should be
> > sorted.
> I got the idea that there was some attempt to group logically before
> alphabetically. Like keeping board/SoC families together. But maybe not.
> 
> I can do as you suggested, if you don't care about keeping actual
> similar boards together (i.e., veyron/3288 vs gru/3399).

I'd prefer a simple alphabetical sorting.

I think people reading the document will know what device they have, but not 
necessarily the actual soc in it. At least I would look for Google Kevin 
primarily without thinking of the soc at first.

And in general, most Rockchip boards (maybe except Google-boards) tend to 
follow the reference design quite closely, so it may become hard to decide 
when one is similar to another :-) . So best to keep it simple.

^ permalink raw reply

* Re: [PATCH v3] rtc: add support for EPSON TOYOCOM RTC-7301SF/DG
From: Alexandre Belloni @ 2016-12-07 19:17 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Alessandro Zummo
In-Reply-To: <1480860279-4570-1-git-send-email-akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 04/12/2016 at 23:04:39 +0900, Akinobu Mita wrote :
> This adds support for EPSON TOYOCOM RTC-7301SF/DG which has parallel
> interface compatible with SRAM.
> 
> This driver supports basic clock, calendar and alarm functionality.
> 
> Tested with Microblaze linux running on Artix7 FPGA board with my own
> custom IP for RTC-7301.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Alessandro Zummo <a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org>
> Cc: Alexandre Belloni <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> * Changes from v2
> - Convert enum to usual #defines
> 
>  .../devicetree/bindings/rtc/epson,rtc7301.txt      |  16 +
>  drivers/rtc/Kconfig                                |  11 +
>  drivers/rtc/Makefile                               |   1 +
>  drivers/rtc/rtc-r7301.c                            | 453 +++++++++++++++++++++
>  4 files changed, 481 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
>  create mode 100644 drivers/rtc/rtc-r7301.c
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [RESEND PATCH v2 4/7] drm/vc4: Add support for the VEC (Video Encoder) IP
From: Eric Anholt @ 2016-12-07 19:18 UTC (permalink / raw)
  To: Boris Brezillon, Florian Fainelli
  Cc: Mark Rutland, devicetree, Ian Campbell, Pawel Moll, Scott Branden,
	Stephen Warren, Ray Jui, Lee Jones, dri-devel, Rob Herring,
	bcm-kernel-feedback-list, linux-rpi-kernel, Kumar Gala,
	linux-arm-kernel
In-Reply-To: <20161207161205.485a3046@bbrezillon>


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

Boris Brezillon <boris.brezillon@free-electrons.com> writes:

> On Mon, 5 Dec 2016 17:50:13 -0800
> Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>> On 12/02/2016 05:48 AM, Boris Brezillon wrote:
>> > The VEC IP is a TV DAC, providing support for PAL and NTSC standards.
>> > 
>> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>> > ---  
>> 
>> > diff --git a/drivers/gpu/drm/vc4/vc4_vec.c b/drivers/gpu/drm/vc4/vc4_vec.c
>> > new file mode 100644
>> > index 000000000000..2d4256fcc6f2
>> > --- /dev/null
>> > +++ b/drivers/gpu/drm/vc4/vc4_vec.c
>> > @@ -0,0 +1,657 @@
>> > +/*
>> > + * Copyright (C) 2016 Broadcom Limited  
>> 
>> The standard copyright template post acquisition is just Broadcom, not
>> Broadcom Limited, nor Broadcom corporation. Can you audit your entire
>> submission and fix this up accordingly?
>
> This is the only patch adding a copyright header.
> Eric, can you fix that when applying the series or should I resend a
> new version?

I'll fix it when applying, that's fine.  (I had in v1 and forgot to tell
you!)

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH V2 4/5] ARM: BCM5301X: Specify all RAM by including an extra block
From: Jon Mason @ 2016-12-07 19:26 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Mark Rutland, devicetree, Florian Fainelli, Arnd Bergmann,
	Hauke Mehrtens, Russell King, linux-kernel, Rob Herring,
	bcm-kernel-feedback-list, Rafał Miłecki,
	linux-arm-kernel
In-Reply-To: <20161207075655.7396-4-zajec5@gmail.com>

On Wed, Dec 07, 2016 at 08:56:54AM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> The first 128 MiB of RAM can be accessed using an alias at address 0x0.
> 
> In theory we could access whole RAM using 0x80000000 - 0xbfffffff range
> (up to 1 GiB) but it doesn't seem to work on Northstar. For some reason
> (hardware setup left by the bootloader maybe?) 0x80000000 - 0x87ffffff
> range can't be used. I reproduced this problem on:
> 1) Buffalo WZR-600DHP2 (BCM47081)
> 2) Netgear R6250 (BCM4708)
> 3) D-Link DIR-885L (BCM47094)
> 
> So it seems we're forced to access first 128 MiB using alias at 0x0 and
> the rest using real base address + 128 MiB offset which is 0x88000000.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Acked-by: Jon Mason <jon.mason@broadcom.com>

> ---
> V2: Updated commit message, thanks Jon!
>     Included XWR-3100
> ---
>  arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts        | 3 ++-
>  arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts        | 3 ++-
>  arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts  | 3 ++-
>  arch/arm/boot/dts/bcm4708-netgear-r6250.dts        | 3 ++-
>  arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts     | 3 ++-
>  arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts      | 3 ++-
>  arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts        | 3 ++-
>  arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts | 3 ++-
>  arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts  | 3 ++-
>  arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts        | 3 ++-
>  arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts  | 3 ++-
>  arch/arm/boot/dts/bcm4709-netgear-r7000.dts        | 3 ++-
>  arch/arm/boot/dts/bcm4709-netgear-r8000.dts        | 3 ++-
>  arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts      | 3 ++-
>  arch/arm/boot/dts/bcm47094-luxul-xwr-3100.dts      | 3 ++-
>  arch/arm/boot/dts/bcm47094-netgear-r8500.dts       | 3 ++-
>  16 files changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> index 112a5a8..d241cee 100644
> --- a/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> +++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac56u.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
> index 3600f56..b0e6204 100644
> --- a/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
> +++ b/arch/arm/boot/dts/bcm4708-asus-rt-ac68u.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
> index d49afec0..c9ba6b9 100644
> --- a/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
> +++ b/arch/arm/boot/dts/bcm4708-buffalo-wzr-1750dhp.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x18000000>;
>  	};
>  
>  	spi {
> diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
> index 8519548..b9f66c0 100644
> --- a/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
> +++ b/arch/arm/boot/dts/bcm4708-netgear-r6250.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> index 6229ef2..ae0199f 100644
> --- a/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> +++ b/arch/arm/boot/dts/bcm4708-netgear-r6300-v2.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts b/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
> index 74cfcd3..36b628b1 100644
> --- a/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
> +++ b/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
> index 71b98cf..db8608b 100644
> --- a/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
> +++ b/arch/arm/boot/dts/bcm47081-asus-rt-n18u.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts b/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
> index 2922536..d51586d 100644
> --- a/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
> +++ b/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	spi {
> diff --git a/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts b/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
> index 184fd92..de041b8 100644
> --- a/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
> +++ b/arch/arm/boot/dts/bcm47081-buffalo-wzr-900dhp.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	gpio-keys {
> diff --git a/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts b/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts
> index eac0f52..eaca687 100644
> --- a/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts
> +++ b/arch/arm/boot/dts/bcm4709-asus-rt-ac87u.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
> index aab39c9..b32957c 100644
> --- a/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
> +++ b/arch/arm/boot/dts/bcm4709-buffalo-wxr-1900dhp.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x18000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm4709-netgear-r7000.dts b/arch/arm/boot/dts/bcm4709-netgear-r7000.dts
> index 7ab1176..f459a98 100644
> --- a/arch/arm/boot/dts/bcm4709-netgear-r7000.dts
> +++ b/arch/arm/boot/dts/bcm4709-netgear-r7000.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
> index 56d38a3..cd13534 100644
> --- a/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
> +++ b/arch/arm/boot/dts/bcm4709-netgear-r8000.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts b/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts
> index 7fb9270..64ded76 100644
> --- a/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts
> +++ b/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dts
> @@ -21,7 +21,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	nand: nand@18028000 {
> diff --git a/arch/arm/boot/dts/bcm47094-luxul-xwr-3100.dts b/arch/arm/boot/dts/bcm47094-luxul-xwr-3100.dts
> index 93cc91d..5cf4ab1 100644
> --- a/arch/arm/boot/dts/bcm47094-luxul-xwr-3100.dts
> +++ b/arch/arm/boot/dts/bcm47094-luxul-xwr-3100.dts
> @@ -18,7 +18,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x08000000>;
>  	};
>  
>  	leds {
> diff --git a/arch/arm/boot/dts/bcm47094-netgear-r8500.dts b/arch/arm/boot/dts/bcm47094-netgear-r8500.dts
> index 7ecd57c..600795e 100644
> --- a/arch/arm/boot/dts/bcm47094-netgear-r8500.dts
> +++ b/arch/arm/boot/dts/bcm47094-netgear-r8500.dts
> @@ -18,7 +18,8 @@
>  	};
>  
>  	memory {
> -		reg = <0x00000000 0x08000000>;
> +		reg = <0x00000000 0x08000000
> +		       0x88000000 0x18000000>;
>  	};
>  
>  	leds {
> -- 
> 2.10.1
> 

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

^ permalink raw reply

* [PATCH v2] ARM: dts: lpc32xx: add pwm-cells to base dts file
From: Sylvain Lemieux @ 2016-12-07 19:30 UTC (permalink / raw)
  To: vz-ChpfBGZJDbMAvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

From: Sylvain Lemieux <slemieux-1xCVI8+nB4ZBDgjK7y7TUQ@public.gmane.org>

There is no need to define the "pwm-cells" in the board
specific dts file; move the entry to the base dts file.

Signed-off-by: Sylvain Lemieux <slemieux-1xCVI8+nB4ZBDgjK7y7TUQ@public.gmane.org>
---
Changes in v2:
* Update the "pwm-cells" to be compatible with the latest
  lpc32xx-pwm driver implementation.

Note:
* This patch have a dependency on the following patcheset:
  "pwm: lpc32xx: switch driver to one phandle argument for PWM consumers"
  http://www.spinics.net/lists/arm-kernel/msg547013.html

 arch/arm/boot/dts/lpc32xx.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi
index 218d9fa..c031c94 100644
--- a/arch/arm/boot/dts/lpc32xx.dtsi
+++ b/arch/arm/boot/dts/lpc32xx.dtsi
@@ -472,6 +472,7 @@
 				assigned-clocks = <&clk LPC32XX_CLK_PWM1>;
 				assigned-clock-parents = <&clk LPC32XX_CLK_PERIPH>;
 				status = "disabled";
+				#pwm-cells = <1>;
 			};
 
 			pwm2: pwm@4005C004 {
@@ -481,6 +482,7 @@
 				assigned-clocks = <&clk LPC32XX_CLK_PWM2>;
 				assigned-clock-parents = <&clk LPC32XX_CLK_PERIPH>;
 				status = "disabled";
+				#pwm-cells = <1>;
 			};
 
 			timer3: timer@40060000 {
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v4 4/5] i2c: designware: Add slave mode as separated driver
From: Andy Shevchenko @ 2016-12-07 19:49 UTC (permalink / raw)
  To: Luis Oliveira, wsa, robh+dt, mark.rutland, jarkko.nikula,
	mika.westerberg, linux-i2c, devicetree, linux-kernel
  Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
In-Reply-To: <a7ca5014ad1c3f4905349a02ebe5294fe64c318e.1481131072.git.lolivei@synopsys.com>

On Wed, 2016-12-07 at 17:55 +0000, Luis Oliveira wrote:
> - Slave mode selected by compatibility string in platform module
> - Changes in Makefile to Kbuild successfully compile i2c-designware-
> core
>   with slave functions

This needs more work.

First of all I would split logically this to two patches:

1. slave patch: introducing header definitions and -slave.c module
2. slave patch: actually use it

Always try to split you changes to smaller logically finished parts.
There is still freedom of choice, though I bet many maintainers will
agree with me.

Last but not least is to keep an eye on bisectability. It means your
each patch should: a) be compiled without problems, b) bring the
features, definitions, whatsoever it uses, c) go with priority — bug
fixes first followed by clean ups followed by feature enhancements.


> Signed-off-by: Luis Oliveira <lolivei@synopsys.com>
> ---
> Changes V3->V4: (Andy Shevchenko)
> - nothing changed

Hmm... I think there were comments about this (perhaps not mine).
Anyway, see below.

 
> +static void i2c_dw_configure_slave(struct platform_device *pdev)
> +{
> +	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
> +
> +	dev->functionality = I2C_FUNC_SLAVE |
> DW_IC_DEFAULT_FUNCTIONALITY;
> +
> +	dev->slave_cfg = DW_IC_CON_RX_FIFO_FULL_HLD_CTRL |
> +			 DW_IC_CON_RESTART_EN |
> DW_IC_CON_STOP_DET_IFADDRESSED |
> +			 DW_IC_CON_SPEED_FAST;
> +

> +	dev_info(&pdev->dev, "I am registed as a I2C Slave!\n");

Same side note as for master case.

> +
> +	switch (dev->clk_freq) {
> +	case 100000:
> +		dev->slave_cfg |= DW_IC_CON_SPEED_STD;
> +		break;
> +	case 3400000:
> +		dev->slave_cfg |= DW_IC_CON_SPEED_HIGH;
> +		break;
> +	default:
> +		dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
> +	}
> +}
> +
>  static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool
> prepare)
>  {
>  	if (IS_ERR(i_dev->clk))
> @@ -240,9 +266,12 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
>  	if (r)
>  		return r;
>  
> -	dev->functionality = I2C_FUNC_10BIT_ADDR |
> DW_IC_DEFAULT_FUNCTIONALITY;
> -
> -	i2c_dw_configure_master(pdev);
> +#ifndef CONFIG_ACPI

This is no-no. And basically you got already comment on it.
Please, fix all occurrences.

> +	if (!device_property_match_string(&pdev->dev, "mode",
> "slave"))
> +		i2c_dw_configure_slave(pdev);
> +	else
> +#endif
> +		i2c_dw_configure_master(pdev);
>  
>  	dev->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (!i2c_dw_plat_prepare_clk(dev, true)) {
> @@ -255,7 +284,14 @@ static int dw_i2c_plat_probe(struct
> platform_device *pdev)
>  	}
>  
>  	if (!dev->tx_fifo_depth) {
> -		u32 param1 = i2c_dw_read_comp_param(dev);
> +		u32 param1;
> +#ifndef CONFIG_ACPI
> +		if (!device_property_match_string(&pdev->dev,
> +			 "mode", "slave"))
> +			param1 = i2c_dw_read_comp_param_slave(dev);
> +		else
> +#endif
> +			param1 = i2c_dw_read_comp_param(dev);
>  
>  		dev->tx_fifo_depth = ((param1 >> 16) & 0xff) + 1;
>  		dev->rx_fifo_depth = ((param1 >> 8)  & 0xff) + 1;

Here is the patch for FIFO flying around and I believe it will make
upstream before yours. Which means try rebase your next version against
latest i2c-next and / or linux-next.

> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-designware-slave.c
> @@ -0,0 +1,433 @@
> +/*

> + * Synopsys DesignWare I2C adapter driver (master only).
> + *
> + * Based on the TI DAVINCI I2C adapter driver.
> + *
> + * Copyright (C) 2006 Texas Instruments.
> + * Copyright (C) 2007 MontaVista Software Inc.
> + * Copyright (C) 2009 Provigent Ltd.

Something wrong here. Perhaps slave for the first?
Why copyrights are kept? Needs explanation.

> + *
> + * ------------------------------------------------------------------
> ----------
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License as published
> by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + * ------------------------------------------------------------------
> ----------
> + *
> + */
> 

> +#include <linux/export.h>

I don't think you need this one explicit. You have already module.h.

> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>

Keep above sorted.

+ empty line here.

+#include "i2c-designware-core.h"
> +
> +static void i2c_dw_configure_fifo_slave(struct dw_i2c_dev *dev)
> +{
> +	/* Configure Tx/Rx FIFO threshold levels */
> +	dw_writel(dev, 0, DW_IC_TX_TL);
> +	dw_writel(dev, 0, DW_IC_RX_TL);
> +
> +	/* configure the I2C slave */
> +	dw_writel(dev, dev->slave_cfg, DW_IC_CON);
> +	dw_writel(dev, DW_IC_INTR_SLAVE_MASK, DW_IC_INTR_MASK);

I understand the copy'n'paste development method, but, please, keep
consistent style in new code, e.g. comments should start from Capital
letter. Multi-line comments have separate open and close lines.

> +}
> +
> +/**
> + * i2c_dw_init_slave() - initialize the designware i2c slave hardware
> + * @dev: device private data
> + *
> + * This functions configures and enables the I2C.
> + * This function is called during I2C init function, and in case of
> timeout at
> + * run time.
> + */
> +int i2c_dw_init_slave(struct dw_i2c_dev *dev)
> +{

> +	u32 hcnt, lcnt;
> +	u32 reg, comp_param1;
> +	u32 sda_falling_time, scl_falling_time;
> +	int ret;

Better to use reversed tree.

aaaaaaaaaaa
bbbbbbb
cccc


> +
> +	ret = i2c_dw_acquire_lock(dev);
> +	if (ret)
> +		return ret;
> +
> +	reg = dw_readl(dev, DW_IC_COMP_TYPE);
> +	if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) {
> +		/* Configure register endianness access */
> +		dev->accessor_flags |= ACCESS_SWAP;
> +	} else if (reg == (DW_IC_COMP_TYPE_VALUE & 0x0000ffff)) {
> +		/* Configure register access mode 16bit */
> +		dev->accessor_flags |= ACCESS_16BIT;
> +	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
> +		dev_err(dev->dev, "Unknown Synopsys component type: "
> +			"0x%08x\n", reg);
> +		i2c_dw_release_lock(dev);
> +		return -ENODEV;
> +	}
> +
> +	comp_param1 = dw_readl(dev, DW_IC_COMP_PARAM_1);
> +
> +	/* Disable the adapter */
> +	__i2c_dw_enable_and_wait(dev, false);
> +
> +	/* set standard and fast speed deviders for high/low periods
> */

Same about comments and style.

> +	sda_falling_time = dev->sda_falling_time ?: 300; /* ns */
> +	scl_falling_time = dev->scl_falling_time ?: 300; /* ns */
> +
> +	/* Set SCL timing parameters for standard-mode */
> +	if (dev->ss_hcnt && dev->ss_lcnt) {
> +		hcnt = dev->ss_hcnt;
> +		lcnt = dev->ss_lcnt;
> +	} else {
> +		hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
> +					4000,	/* tHD;STA =
> tHIGH = 4.0 us */
> +					sda_falling_time,
> +					0,	/* 0: DW default,
> 1: Ideal */
> +					0);	/* No offset */
> +		lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
> +					4700,	/* tLOW = 4.7 us
> */
> +					scl_falling_time,
> +					0);	/* No offset */
> +	}
> +	dw_writel(dev, hcnt, DW_IC_SS_SCL_HCNT);
> +	dw_writel(dev, lcnt, DW_IC_SS_SCL_LCNT);
> +	dev_dbg(dev->dev, "Standard-mode HCNT:LCNT = %d:%d\n", hcnt,
> lcnt);
> +
> +	/* Set SCL timing parameters for fast-mode or fast-mode plus
> */
> +	if ((dev->clk_freq == 1000000) && dev->fp_hcnt && dev-
> >fp_lcnt) {
> +		hcnt = dev->fp_hcnt;
> +		lcnt = dev->fp_lcnt;
> +	} else if (dev->fs_hcnt && dev->fs_lcnt) {
> +		hcnt = dev->fs_hcnt;
> +		lcnt = dev->fs_lcnt;
> +	} else {
> +		hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev),
> +					600,	/* tHD;STA =
> tHIGH = 0.6 us */
> +					sda_falling_time,
> +					0,	/* 0: DW default,
> 1: Ideal */
> +					0);	/* No offset */
> +		lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev),
> +					1300,	/* tLOW = 1.3 us
> */
> +					scl_falling_time,
> +					0);	/* No offset */
> +	}

I didn't read carefully, but on the first glance how does it differ from
master code?

If it doesn't, split the original function of the master (better in
separate patch) to few to prepare for slave.

> +	dw_writel(dev, hcnt, DW_IC_FS_SCL_HCNT);
> +	dw_writel(dev, lcnt, DW_IC_FS_SCL_LCNT);
> +	dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt,
> lcnt);
> +
> +	if ((dev->slave_cfg & DW_IC_CON_SPEED_MASK) ==
> +		DW_IC_CON_SPEED_HIGH) {
> +		if ((comp_param1 &
> DW_IC_COMP_PARAM_1_SPEED_MODE_MASK)
> +			!= DW_IC_COMP_PARAM_1_SPEED_MODE_HIGH) {
> +			dev_err(dev->dev, "High Speed not
> supported!\n");
> +			dev->slave_cfg &= ~DW_IC_CON_SPEED_MASK;
> +			dev->slave_cfg |= DW_IC_CON_SPEED_FAST;
> +		} else if (dev->hs_hcnt && dev->hs_lcnt) {
> +			hcnt = dev->hs_hcnt;
> +			lcnt = dev->hs_lcnt;
> +			dw_writel(dev, hcnt, DW_IC_HS_SCL_HCNT);
> +			dw_writel(dev, lcnt, DW_IC_HS_SCL_LCNT);
> +			dev_dbg(dev->dev, "HighSpeed-mode HCNT:LCNT =
> %d:%d\n",
> +				hcnt, lcnt);
> +		}
> +	}
> +
> +	/* Configure SDA Hold Time if required */
> +	reg = dw_readl(dev, DW_IC_COMP_VERSION);
> +	reg = dw_readl(dev, DW_IC_COMP_VERSION);
> +	if (reg >= DW_IC_SDA_HOLD_MIN_VERS) {
> +		if (!dev->sda_hold_time) {
> +			/* Keep previous hold time setting if no one
> set it */
> +			dev->sda_hold_time = dw_readl(dev,
> DW_IC_SDA_HOLD);
> +		}
> +		/*
> +		 * Workaround for avoiding TX arbitration lost in
> case I2C
> +		 * slave pulls SDA down "too quickly" after falling
> egde of
> +		 * SCL by enabling non-zero SDA RX hold.
> Specification says it
> +		 * extends incoming SDA low to high transition while
> SCL is
> +		 * high but it apprears to help also above issue.
> +		 */
> +		if (!(dev->sda_hold_time & DW_IC_SDA_HOLD_RX_MASK))
> +			dev->sda_hold_time |= 1 <<
> DW_IC_SDA_HOLD_RX_SHIFT;
> +		dw_writel(dev, dev->sda_hold_time, DW_IC_SDA_HOLD);
> +	} else {
> +		dev_warn(dev->dev,
> +			"Hardware too old to adjust SDA hold
> time.\n");
> +	}
> +
> +	i2c_dw_configure_fifo_slave(dev);
> +	i2c_dw_release_lock(dev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_dw_init_slave);
> +
> +int i2c_dw_reg_slave(struct i2c_client *slave)
> +{
> +	struct dw_i2c_dev *dev = i2c_get_adapdata(slave->adapter);
> +
> +	if (dev->slave)
> +		return -EBUSY;
> +	if (slave->flags & I2C_CLIENT_TEN)
> +		return -EAFNOSUPPORT;
> +		/* set slave address in the IC_SAR register,
> +		* the address to which the DW_apb_i2c responds */
> +
> +	__i2c_dw_enable(dev, false);
> +	dw_writel(dev, slave->addr, DW_IC_SAR);
> +	dev->slave = slave;
> +
> +	__i2c_dw_enable(dev, true);
> +
> +	dev->cmd_err = 0;
> +	dev->msg_write_idx = 0;
> +	dev->msg_read_idx = 0;
> +	dev->msg_err = 0;
> +	dev->status = STATUS_IDLE;
> +	dev->abort_source = 0;
> +	dev->rx_outstanding = 0;
> +
> +	return 0;
> +}
> +
> +static int i2c_dw_unreg_slave(struct i2c_client *slave)
> +{
> +	struct dw_i2c_dev *dev =  i2c_get_adapdata(slave->adapter);
> +
> +	i2c_dw_disable_int_slave(dev);
> +	i2c_dw_disable_slave(dev);
> +	dev->slave =  NULL;
> +
> +	return 0;
> +}
> +
> +static u32 i2c_dw_read_clear_intrbits_slave(struct dw_i2c_dev *dev)
> +{
> +	u32 stat;
> +
> +	/*
> +	 * The IC_INTR_STAT register just indicates "enabled"
> interrupts.
> +	 * Ths unmasked raw version of interrupt status bits are
> available
> +	 * in the IC_RAW_INTR_STAT register.
> +	 *
> +	 * That is,
> +	 *   stat = dw_readl(IC_INTR_STAT);
> +	 * equals to,
> +	 *   stat = dw_readl(IC_RAW_INTR_STAT) &
> dw_readl(IC_INTR_MASK);
> +	 *
> +	 * The raw version might be useful for debugging purposes.
> +	 */
> +	stat = dw_readl(dev, DW_IC_INTR_STAT);
> +
> +	/*
> +	 * Do not use the IC_CLR_INTR register to clear interrupts,
> or
> +	 * you'll miss some interrupts, triggered during the period
> from
> +	 * dw_readl(IC_INTR_STAT) to dw_readl(IC_CLR_INTR).
> +	 *
> +	 * Instead, use the separately-prepared IC_CLR_* registers.
> +	 */
> +	if (stat & DW_IC_INTR_TX_ABRT)
> +		dw_readl(dev, DW_IC_CLR_TX_ABRT);
> +	if (stat & DW_IC_INTR_RX_UNDER)
> +		dw_readl(dev, DW_IC_CLR_RX_UNDER);
> +	if (stat & DW_IC_INTR_RX_OVER)
> +		dw_readl(dev, DW_IC_CLR_RX_OVER);
> +	if (stat & DW_IC_INTR_TX_OVER)
> +		dw_readl(dev, DW_IC_CLR_TX_OVER);
> +	if (stat & DW_IC_INTR_RX_DONE)
> +		dw_readl(dev, DW_IC_CLR_RX_DONE);
> +	if (stat & DW_IC_INTR_ACTIVITY)
> +		dw_readl(dev, DW_IC_CLR_ACTIVITY);
> +	if (stat & DW_IC_INTR_STOP_DET)
> +		dw_readl(dev, DW_IC_CLR_STOP_DET);
> +	if (stat & DW_IC_INTR_START_DET)
> +		dw_readl(dev, DW_IC_CLR_START_DET);
> +	if (stat & DW_IC_INTR_GEN_CALL)
> +		dw_readl(dev, DW_IC_CLR_GEN_CALL);
> +
> +	return stat;
> +}
> +
> +/*
> + * Interrupt service routine. This gets called whenever an I2C slave
> interrupt
> + * occurs.
> + */
> +
> +static bool i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)

static int
Choose 0 for IRQ_NONE, negative for error (if needed, perhaps not your
case), positive for IRQ_HANDLED case.

> +{
> +	u32 raw_stat, stat, enabled;
> +	u8 val, slave_activity;
> +
> +	stat = dw_readl(dev, DW_IC_INTR_STAT);
> +	enabled = dw_readl(dev, DW_IC_ENABLE);
> +	raw_stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
> +	slave_activity = ((dw_readl(dev, DW_IC_STATUS) &
> +		 DW_IC_STATUS_SLAVE_ACTIVITY)>>6);

x >> y

style.

> +
> +	if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY))
> +		return false;
> +
> +	dev_dbg(dev->dev,
> +	 "%s: %#x SLAVE_ACTV=%#x : RAW_INTR_STAT=%#x :
> INTR_STAT=%#x\n",
> +	 __func__, enabled, slave_activity, raw_stat, stat);

__func__ is redundant in *_dbg() calls. Dynamic debug could do it for
you.

> +
> +	if (stat & DW_IC_INTR_RESTART_DET)
> +		dw_readl(dev, DW_IC_CLR_RESTART_DET);
> +	if (stat & DW_IC_INTR_START_DET)
> +		dw_readl(dev, DW_IC_CLR_START_DET);
> +	if (stat & DW_IC_INTR_ACTIVITY)
> +		dw_readl(dev, DW_IC_CLR_ACTIVITY);
> +	if (stat & DW_IC_INTR_RX_OVER)
> +		dw_readl(dev, DW_IC_CLR_RX_OVER);
> +	if ((stat & DW_IC_INTR_RX_FULL) && (stat &
> DW_IC_INTR_STOP_DET))
> +		i2c_slave_event(dev->slave,
> I2C_SLAVE_WRITE_REQUESTED, &val);
> +
> +	if (slave_activity) {
> +		if (stat & DW_IC_INTR_RD_REQ) {
> +			if (stat & DW_IC_INTR_RX_FULL) {
> +				val = dw_readl(dev, DW_IC_DATA_CMD);
> +				if (!i2c_slave_event(dev->slave,
> +				 I2C_SLAVE_WRITE_RECEIVED, &val)) {
> +					dev_dbg(dev->dev, "Byte %X
> acked!",
> +					 val);
> +				}
> +				dw_readl(dev, DW_IC_CLR_RD_REQ);
> +				stat =
> i2c_dw_read_clear_intrbits_slave(dev);
> +			} else {
> +				dw_readl(dev, DW_IC_CLR_RD_REQ);
> +				dw_readl(dev, DW_IC_CLR_RX_UNDER);
> +				stat =
> i2c_dw_read_clear_intrbits_slave(dev);
> +			}
> +			if (!i2c_slave_event(dev->slave,
> +					 I2C_SLAVE_READ_REQUESTED,
> &val))
> +				dw_writel(dev, val, DW_IC_DATA_CMD);
> +		}
> +	}
> +
> +	if (stat & DW_IC_INTR_RX_DONE) {
> +		if (!i2c_slave_event(dev->slave,
> I2C_SLAVE_READ_PROCESSED,
> +		 &val))
> +			dw_readl(dev, DW_IC_CLR_RX_DONE);
> +
> +		i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
> +		stat = i2c_dw_read_clear_intrbits_slave(dev);
> +		return true;
> +	}
> +
> +	if (stat & DW_IC_INTR_RX_FULL) {
> +		val = dw_readl(dev, DW_IC_DATA_CMD);
> +		if (!i2c_slave_event(dev->slave,
> I2C_SLAVE_WRITE_RECEIVED,
> +		 &val))
> +			dev_dbg(dev->dev, "Byte %X acked!", val);
> +	} else {
> +		i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
> +		stat = i2c_dw_read_clear_intrbits_slave(dev);
> +	}
> +
> +	if (stat & DW_IC_INTR_TX_OVER)
> +		dw_readl(dev, DW_IC_CLR_TX_OVER);
> +
> +	return true;
> +}
> +
> +static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
> +{
> +	struct dw_i2c_dev *dev = dev_id;
> +
> +	i2c_dw_read_clear_intrbits_slave(dev);
> +	if (!i2c_dw_irq_handler_slave(dev))
> +		return IRQ_NONE;
> +
> +	complete(&dev->cmd_complete);
> +	return IRQ_HANDLED;

...and here (if you want to, this might be kept as is for now, but still
would be nice to get int return code anyway)

int ret;

ret = i2c_dw_irq_handler_slave(dev);
if (ret > 0) {
 complete(...);
}

return IRQ_RETVAL(ret);

> +}
> +
> +static struct i2c_algorithm i2c_dw_algo = {
> +	.functionality	= i2c_dw_func,
> +	.reg_slave	= i2c_dw_reg_slave,
> +	.unreg_slave	= i2c_dw_unreg_slave,
> +};
> +
> +void i2c_dw_disable_slave(struct dw_i2c_dev *dev)
> +{
> +	/* Disable controller */
> +	__i2c_dw_enable_and_wait(dev, false);
> +
> +	/* Disable all interupts */
> +	dw_writel(dev, 0, DW_IC_INTR_MASK);
> +	dw_readl(dev, DW_IC_CLR_INTR);
> +}
> +EXPORT_SYMBOL_GPL(i2c_dw_disable_slave);

No, please use
struct i2c_dw_ops {
 (*func1)();
 (*func2)();
...
};

for both (master/slave).

> +
> +void i2c_dw_disable_int_slave(struct dw_i2c_dev *dev)
> +{
> +	dw_writel(dev, 0, DW_IC_INTR_MASK);
> +}
> +EXPORT_SYMBOL_GPL(i2c_dw_disable_int_slave);
> +
> +u32 i2c_dw_read_comp_param_slave(struct dw_i2c_dev *dev)
> +{
> +	return dw_readl(dev, DW_IC_COMP_PARAM_1);
> +}
> +EXPORT_SYMBOL_GPL(i2c_dw_read_comp_param_slave);
> +
> +int i2c_dw_probe_slave(struct dw_i2c_dev *dev)
> +{
> +	struct i2c_adapter *adap = &dev->adapter;

> +	int r;

New code — consistency.

int ret; as above.

> +
> +	init_completion(&dev->cmd_complete);
> +
> +	r = i2c_dw_init_slave(dev);
> +	if (r)
> +		return r;
> +
> +	r = i2c_dw_acquire_lock(dev);
> +	if (r)
> +		return r;
> +
> +	i2c_dw_release_lock(dev);
> +	snprintf(adap->name, sizeof(adap->name),
> +		 "Synopsys DesignWare I2C Slave adapter");
> +	adap->retries = 3;
> +	adap->algo = &i2c_dw_algo;
> +	adap->dev.parent = dev->dev;
> +	i2c_set_adapdata(adap, dev);
> +
> +	r = devm_request_irq(dev->dev, dev->irq, i2c_dw_isr_slave,
> 

> +			     IRQF_SHARED | IRQF_COND_SUSPEND,

I don't think IRQF_COND_SUSPEND makes sense in slave mode (I know
exactly one user of it and I ensure you that it will quite unlikely use
slave mode), though it will not block others anyhow.

> +			     dev_name(dev->dev), dev);
> +	if (r) {
> +		dev_err(dev->dev, "failure requesting irq %i: %d\n",
> +			dev->irq, r);
> +		return r;
> +	}
> +	/*
> +	 * Increment PM usage count during adapter registration in
> order to
> +	 * avoid possible spurious runtime suspend when adapter
> device is
> +	 * registered to the device core and immediate resume in case
> bus has
> +	 * registered I2C slaves that do I2C transfers in their
> probe.
> +	 */
> +	pm_runtime_get_noresume(dev->dev);
> +	r = i2c_add_numbered_adapter(adap);
> +	if (r)
> +		dev_err(dev->dev, "failure adding adapter: %d\n", r);
> +	pm_runtime_put_noidle(dev->dev);
> +
> +	return r;
> +}
> +EXPORT_SYMBOL_GPL(i2c_dw_probe_slave);
> +
> +MODULE_DESCRIPTION("Synopsys DesignWare I2C bus slave adapter");
> +MODULE_LICENSE("GPL");

"GPL v2"?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH v4 5/5] i2c: designware: Cleaning comments and formatation
From: Andy Shevchenko @ 2016-12-07 19:52 UTC (permalink / raw)
  To: Luis Oliveira, wsa, robh+dt, mark.rutland, jarkko.nikula,
	mika.westerberg, linux-i2c, devicetree, linux-kernel
  Cc: Ramiro.Oliveira, Joao.Pinto, CARLOS.PALMINHA
In-Reply-To: <02856fdb6ce3230a4ac1ba0958938ad51e763205.1481131072.git.lolivei@synopsys.com>

On Wed, 2016-12-07 at 17:55 +0000, Luis Oliveira wrote:
> - Missspelling, comment formatation and fix a string of
>   the existing code
> 

Good, but after addressing below comments:
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> --- a/drivers/i2c/busses/i2c-designware-slave.c
> +++ b/drivers/i2c/busses/i2c-designware-slave.c
> @@ -70,8 +70,8 @@ int i2c_dw_init_slave(struct dw_i2c_dev *dev)
>  		/* Configure register access mode 16bit */
>  		dev->accessor_flags |= ACCESS_16BIT;
>  	} else if (reg != DW_IC_COMP_TYPE_VALUE) {
> -		dev_err(dev->dev, "Unknown Synopsys component type: "
> -			"0x%08x\n", reg);
> +		dev_err(dev->dev,
> +		 "Unknown Synopsys component type: 0x%08x\n", reg);

Keep indentation like 

some_call(param, param2, ...
	  paramX, paramY, ...);

It applies to all your new code you bring by this series.


> @@ -181,8 +181,10 @@ int i2c_dw_reg_slave(struct i2c_client *slave)
>  		return -EBUSY;
>  	if (slave->flags & I2C_CLIENT_TEN)
>  		return -EAFNOSUPPORT;
> -		/* set slave address in the IC_SAR register,
> -		* the address to which the DW_apb_i2c responds */
> +		/*
> +		 * set slave address in the IC_SAR register,
> +		 * the address to which the DW_apb_i2c responds

Again, Capital letter here, dot at the end of sentence. You may fix (if
you want to) the rest of comment lines in entire driver.

> +		 */
>  
>  	__i2c_dw_enable(dev, false);
>  	dw_writel(dev, slave->addr, DW_IC_SAR);

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* [PATCH 1/2] ASoC: Add support for CS43130 codec
From: Li Xu @ 2016-12-07 20:17 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	brian.austin-jGc1dHjMKG3QT0dZR+AlfA,
	Paul.Handrigan-jGc1dHjMKG3QT0dZR+AlfA, Li Xu

Add support for Cirrus Logic CS43130 codec.
Support I2C control and I2S audio playback.

Signed-off-by: Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>
---
 sound/soc/codecs/Kconfig   |    6 +
 sound/soc/codecs/Makefile  |    2 +
 sound/soc/codecs/cs43130.c | 1106 ++++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/cs43130.h |  268 +++++++++++
 4 files changed, 1382 insertions(+)
 create mode 100644 sound/soc/codecs/cs43130.c
 create mode 100644 sound/soc/codecs/cs43130.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 9e1718a..c07bc0d 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -62,6 +62,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_CS4349 if I2C
 	select SND_SOC_CS47L24 if MFD_CS47L24
 	select SND_SOC_CS53L30 if I2C
+	select SND_SOC_CS43130 if I2C
 	select SND_SOC_CX20442 if TTY
 	select SND_SOC_DA7210 if SND_SOC_I2C_AND_SPI
 	select SND_SOC_DA7213 if I2C
@@ -486,6 +487,11 @@ config SND_SOC_CS53L30
 	tristate "Cirrus Logic CS53L30 CODEC"
 	depends on I2C
 
+# Cirrus Logic CS43130 HiFi DAC
+config SND_SOC_CS43130
+        tristate "Cirrus Logic CS43130 CODEC"
+        depends on I2C
+
 config SND_SOC_CX20442
 	tristate
 	depends on TTY
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 7e1dad7..7e41526 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -55,6 +55,7 @@ snd-soc-cs42xx8-i2c-objs := cs42xx8-i2c.o
 snd-soc-cs4349-objs := cs4349.o
 snd-soc-cs47l24-objs := cs47l24.o
 snd-soc-cs53l30-objs := cs53l30.o
+snd-soc-cs43130-objs := cs43130.o
 snd-soc-cx20442-objs := cx20442.o
 snd-soc-da7210-objs := da7210.o
 snd-soc-da7213-objs := da7213.o
@@ -284,6 +285,7 @@ obj-$(CONFIG_SND_SOC_CS42XX8_I2C) += snd-soc-cs42xx8-i2c.o
 obj-$(CONFIG_SND_SOC_CS4349)	+= snd-soc-cs4349.o
 obj-$(CONFIG_SND_SOC_CS47L24)	+= snd-soc-cs47l24.o
 obj-$(CONFIG_SND_SOC_CS53L30)	+= snd-soc-cs53l30.o
+obj-$(CONFIG_SND_SOC_CS43130)   += snd-soc-cs43130.o
 obj-$(CONFIG_SND_SOC_CX20442)	+= snd-soc-cx20442.o
 obj-$(CONFIG_SND_SOC_DA7210)	+= snd-soc-da7210.o
 obj-$(CONFIG_SND_SOC_DA7213)	+= snd-soc-da7213.o
diff --git a/sound/soc/codecs/cs43130.c b/sound/soc/codecs/cs43130.c
new file mode 100644
index 0000000..cf1c60c
--- /dev/null
+++ b/sound/soc/codecs/cs43130.c
@@ -0,0 +1,1106 @@
+/*
+ * cs43130.c  --  CS43130 ALSA Soc Audio driver
+ *
+ * Copyright 2016 Cirrus Logic, Inc.
+ *
+ * Authors: Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/i2c.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <sound/core.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+#include <sound/initval.h>
+#include <sound/tlv.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_irq.h>
+
+#include "cs43130.h"
+
+
+static const struct reg_default cs43130_reg_defaults[] = {
+	{ CS43130_SYS_CLK_CTL_1, 0x06 },
+	{ CS43130_SP_SRATE, 0x01 },
+	{ CS43130_SP_BITSIZE, 0x05 },
+	{ CS43130_PAD_INT_CFG, 0x03 },
+	{ CS43130_PWDN_CTL, 0xFE },
+	{ CS43130_CRYSTAL_SET, 0x04 },
+	{ CS43130_PLL_SET_1, 0x00 },
+	{ CS43130_PLL_SET_2, 0x00 },
+	{ CS43130_PLL_SET_3, 0x00 },
+	{ CS43130_PLL_SET_4, 0x00 },
+	{ CS43130_PLL_SET_5, 0x40 },
+	{ CS43130_PLL_SET_6, 0x10 },
+	{ CS43130_PLL_SET_7, 0x80 },
+	{ CS43130_PLL_SET_8, 0x03 },
+	{ CS43130_PLL_SET_9, 0x02 },
+	{ CS43130_PLL_SET_10, 0x02 },
+	{ CS43130_CLKOUT_CTL, 0x00 },
+	{ CS43130_ASP_NUM_1, 0x01 },
+	{ CS43130_ASP_NUM_2, 0x00 },
+	{ CS43130_ASP_DENOM_1, 0x08 },
+	{ CS43130_ASP_DENOM_2, 0x00 },
+	{ CS43130_ASP_LRCK_HI_TIME_1, 0x1F },
+	{ CS43130_ASP_LRCK_HI_TIME_2, 0x00 },
+	{ CS43130_ASP_LRCK_PERIOD_1, 0x3F },
+	{ CS43130_ASP_LRCK_PERIOD_2, 0x00 },
+	{ CS43130_ASP_CLOCK_CONF, 0x0C },
+	{ CS43130_ASP_FRAME_CONF, 0x0A },
+	{ CS43130_XSP_NUM_1, 0x01 },
+	{ CS43130_XSP_NUM_2, 0x00 },
+	{ CS43130_XSP_DENOM_1, 0x02 },
+	{ CS43130_XSP_DENOM_2, 0x00 },
+	{ CS43130_XSP_LRCK_HI_TIME_1, 0x1F },
+	{ CS43130_XSP_LRCK_HI_TIME_2, 0x00 },
+	{ CS43130_XSP_LRCK_PERIOD_1, 0x3F },
+	{ CS43130_XSP_LRCK_PERIOD_2, 0x00 },
+	{ CS43130_XSP_CLOCK_CONF, 0x0C },
+	{ CS43130_XSP_FRAME_CONF, 0x0A },
+	{ CS43130_ASP_CH_1_LOC, 0x00 },
+	{ CS43130_ASP_CH_2_LOC, 0x00 },
+	{ CS43130_ASP_CH_1_SZ_EN, 0x06 },
+	{ CS43130_ASP_CH_2_SZ_EN, 0x0E },
+	{ CS43130_XSP_CH_1_LOC, 0x00 },
+	{ CS43130_XSP_CH_2_LOC, 0x00 },
+	{ CS43130_XSP_CH_1_SZ_EN, 0x06 },
+	{ CS43130_XSP_CH_2_SZ_EN, 0x0E },
+	{ CS43130_DSD_VOL_B, 0x78 },
+	{ CS43130_DSD_VOL_A, 0x78 },
+	{ CS43130_DSD_PATH_CTL_1, 0xA8 },
+	{ CS43130_DSD_INT_CFG, 0x00 },
+	{ CS43130_DSD_PATH_CTL_2, 0x00 },
+	{ CS43130_DSD_PCM_MIX_CTL, 0x00 },
+	{ CS43130_DSD_PATH_CTL_3, 0x40 },
+	{ CS43130_HP_OUT_CTL_1, 0x30 },
+	{ CS43130_PCM_FILT_OPT, 0x02 },
+	{ CS43130_PCM_VOL_B, 0x78 },
+	{ CS43130_PCM_VOL_A, 0x78 },
+	{ CS43130_PCM_PATH_CTL_1, 0xA8 },
+	{ CS43130_PCM_PATH_CTL_2, 0x00 },
+	{ CS43130_CLASS_H_CTL, 0x1E },
+	{ CS43130_HP_DETECT, 0x04 },
+	{ CS43130_HP_LOAD_1, 0x00 },
+	{ CS43130_HP_MEAS_LOAD_1, 0x00 },
+	{ CS43130_HP_MEAS_LOAD_2, 0x00 },
+	{ CS43130_INT_MASK_1, 0xFF },
+	{ CS43130_INT_MASK_2, 0xFF },
+	{ CS43130_INT_MASK_3, 0xFF },
+	{ CS43130_INT_MASK_4, 0xFF },
+	{ CS43130_INT_MASK_5, 0xFF },
+};
+
+static bool cs43130_volatile_register(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case CS43130_DEVID_AB ... CS43130_SUBREV_ID:
+	case CS43130_INT_STATUS_1 ... CS43130_INT_STATUS_5:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool cs43130_readable_register(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case CS43130_DEVID_AB ... CS43130_SYS_CLK_CTL_1:
+	case CS43130_SP_SRATE ... CS43130_PAD_INT_CFG:
+	case CS43130_DXD1:
+	case CS43130_PWDN_CTL:
+	case CS43130_DXD2:
+	case CS43130_CRYSTAL_SET:
+	case CS43130_PLL_SET_1 ... CS43130_PLL_SET_5:
+	case CS43130_PLL_SET_6:
+	case CS43130_PLL_SET_7:
+	case CS43130_PLL_SET_8:
+	case CS43130_PLL_SET_9:
+	case CS43130_PLL_SET_10:
+	case CS43130_CLKOUT_CTL:
+	case CS43130_ASP_NUM_1 ... CS43130_ASP_FRAME_CONF:
+	case CS43130_XSP_NUM_1 ... CS43130_XSP_FRAME_CONF:
+	case CS43130_ASP_CH_1_LOC:
+	case CS43130_ASP_CH_2_LOC:
+	case CS43130_ASP_CH_1_SZ_EN:
+	case CS43130_ASP_CH_2_SZ_EN:
+	case CS43130_XSP_CH_1_LOC:
+	case CS43130_XSP_CH_2_LOC:
+	case CS43130_XSP_CH_1_SZ_EN:
+	case CS43130_XSP_CH_2_SZ_EN:
+	case CS43130_DSD_VOL_B ... CS43130_DSD_PATH_CTL_3:
+	case CS43130_HP_OUT_CTL_1:
+	case CS43130_PCM_FILT_OPT ... CS43130_PCM_PATH_CTL_2:
+	case CS43130_CLASS_H_CTL:
+	case CS43130_HP_DETECT:
+	case CS43130_HP_STATUS:
+	case CS43130_HP_LOAD_1:
+	case CS43130_HP_MEAS_LOAD_1:
+	case CS43130_HP_MEAS_LOAD_2:
+	case CS43130_HP_DC_STAT_1:
+	case CS43130_HP_DC_STAT_2:
+	case CS43130_HP_AC_STAT_1:
+	case CS43130_HP_AC_STAT_2:
+	case CS43130_HP_LOAD_STAT:
+	case CS43130_INT_STATUS_1 ... CS43130_INT_STATUS_5:
+	case CS43130_INT_MASK_1 ... CS43130_INT_MASK_5:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool cs43130_precious_register(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case CS43130_INT_STATUS_1 ... CS43130_INT_STATUS_5:
+		return true;
+	default:
+		return false;
+	}
+}
+
+struct cs43130_pll_params {
+	u32 pll_in;
+	u8 mclk_int;
+	u8 sclk_prediv;
+	u8 pll_div_int;
+	u32 pll_div_frac;
+	u8 pll_mode;
+	u8 pll_divout;
+	u32 pll_out;
+	u8 pll_cal_ratio;
+};
+
+static const struct cs43130_pll_params pll_ratio_table[] = {
+	{ 9600000, 1, 0x02, 0x49, 0x800000, 0x00, 0x08, 22579200, 151 },
+	{ 9600000, 0, 0x02, 0x50, 0x000000, 0x00, 0x08, 24576000, 164 },
+
+	{ 11289600, 1, 0x02, 0X40, 0, 0x01, 0x08, 22579200, 128 },
+	{ 11289600, 0, 0x02, 0x44, 0x06F700, 0x0, 0x08, 24576000, 139 },
+
+	{ 12000000, 1, 0x02, 0x49, 0x800000, 0x00, 0x0A, 22579200, 120 },
+	{ 12000000, 0, 0x02, 0x40, 0x000000, 0x00, 0x08, 24576000, 131 },
+
+	{ 12288000, 1, 0x02, 0x49, 0x800000, 0x01, 0x0A, 22579200, 118 },
+	{ 12288000, 0, 0x02, 0x40, 0x000000, 0x01, 0x08, 24576000, 128 },
+
+	{ 13000000, 1, 0x02, 0x49, 0x800000, 0x01, 0x0A, 22579200, 118 },
+	{ 13000000, 0, 0x02, 0x40, 0x000000, 0x01, 0x08, 24576000, 128 },
+
+	{ 19200000, 1, 0x02, 0x45, 0x797680, 0x01, 0x0A, 22579200, 111 },
+	{ 19200000, 0, 0x02, 0x3C, 0x7EA940, 0x01, 0x08, 24576000, 121 },
+
+	{ 22579200, 1, 0, 0, 0, 0, 0, 22579200, 0 },
+	{ 22579200, 0, 0x03, 0x44, 0x06F700, 0x00, 0x08, 24576000, 139 },
+
+	{ 24000000, 1, 0x03, 0x49, 0x800000, 0x00, 0x0A, 22579200, 120 },
+	{ 24000000, 0, 0x03, 0x40, 0x000000, 0x00, 0x08, 24576000, 131 },
+
+	{ 24576000, 1, 0x03, 0x49, 0x800000, 0x01, 0x0A, 22579200, 128 },
+	{ 24576000, 0, 0, 0, 0, 0, 0, 24576000, 0 },
+
+	{ 26000000, 1, 0x03, 0x45, 0x797680, 0x01, 0x0A, 22579200, 111 },
+	{ 26000000, 0, 0x03, 0x3C, 0x7EA940, 0x01, 0x08, 24576000, 121 },
+};
+
+static int cs43130_pll_config(struct snd_soc_codec *codec)
+{
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+	int i;
+
+	dev_dbg(codec->dev, "%s: cs43130->mclk = %d, cs43130->pll_out = %d",
+		__func__, cs43130->mclk, cs43130->pll_out);
+	for (i = 0; i < ARRAY_SIZE(pll_ratio_table); i++) {
+		if (pll_ratio_table[i].pll_in == cs43130->mclk &&
+			pll_ratio_table[i].pll_out == cs43130->pll_out) {
+
+			cs43130->mclk_int = pll_ratio_table[i].mclk_int;
+
+			if (pll_ratio_table[i].pll_cal_ratio == 0) {
+				if (cs43130->xtal_ibias > 0) {
+					usleep_range(1000, 1050);
+					/*PDN_XTAL = 0,enable*/
+					regmap_update_bits(cs43130->regmap,
+						CS43130_PWDN_CTL,
+						CS43130_PDN_XTAL_MASK, 0
+						<< CS43130_PDN_XTAL_SHIFT);
+				}
+
+				/* PLL_START = 0, disable PLL_START */
+				regmap_update_bits(cs43130->regmap,
+					CS43130_PLL_SET_1,
+					CS43130_PLL_START_MASK,
+				    0 << CS43130_PLL_START_MASK);
+
+				cs43130->pll_bypass = true;
+				return 0;
+			}
+
+			/*PDN_PLL= 0,enable*/
+			regmap_update_bits(cs43130->regmap, CS43130_PWDN_CTL,
+						    CS43130_PDN_PLL_MASK, 0
+						   << CS43130_PDN_PLL_SHIFT);
+
+			regmap_update_bits(cs43130->regmap, CS43130_PLL_SET_9,
+					    CS43130_PLL_REF_PREDIV_MASK,
+						pll_ratio_table[i].sclk_prediv
+						);
+
+			regmap_write(cs43130->regmap, CS43130_PLL_SET_5,
+				pll_ratio_table[i].pll_div_int);
+
+			regmap_write(cs43130->regmap, CS43130_PLL_SET_2,
+				pll_ratio_table[i].pll_div_frac &
+					CS43130_7_0_MASK);
+
+			regmap_write(cs43130->regmap, CS43130_PLL_SET_3,
+				(pll_ratio_table[i].pll_div_frac &
+					CS43130_15_8_MASK) >> 8);
+
+			regmap_write(cs43130->regmap, CS43130_PLL_SET_4,
+				(pll_ratio_table[i].pll_div_frac &
+					CS43130_23_16_MASK) >> 16);
+
+			regmap_update_bits(cs43130->regmap, CS43130_PLL_SET_8,
+					    CS43130_PLL_MODE_MASK,
+						pll_ratio_table[i].pll_mode
+						<< CS43130_PLL_MODE_SHIFT);
+
+			regmap_write(cs43130->regmap, CS43130_PLL_SET_6,
+				pll_ratio_table[i].pll_divout);
+
+			regmap_write(cs43130->regmap, CS43130_PLL_SET_7,
+				pll_ratio_table[i].pll_cal_ratio);
+
+			/* PLL_START = 1, enable PLL_START */
+			regmap_update_bits(cs43130->regmap, CS43130_PLL_SET_1,
+						    CS43130_PLL_START_MASK,
+						    CS43130_PLL_START_MASK);
+			cs43130->pll_bypass = false;
+			return 0;
+		}
+	}
+	return -EINVAL;
+}
+
+static int cs43130_format_config(struct snd_soc_codec *codec)
+{
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	int period_size = 0;
+	int pulse_width = 0;
+	int asp_fsd;
+	int asp_stp;
+	int bick_inv;
+	int asp_m = 0;
+	int asp_sprate = 0;
+	int ret = 0;
+	unsigned int bitwidth_sclk = (cs43130->sclk / cs43130->fs) / 2;
+	unsigned int bitwidth_dai = (cs43130->dai_bit + 1) * 8;
+
+	if (cs43130->fs) {
+		if (bitwidth_sclk < bitwidth_dai) {
+			dev_err(codec->dev, "Format not supported\n");
+			return -EINVAL;
+		}
+		period_size = cs43130->sclk / cs43130->fs;
+		pulse_width = period_size/2;
+
+		if (cs43130->sclk != 0)
+			asp_m = cs43130->pll_out / cs43130->sclk;
+	}
+	dev_dbg(codec->dev, "%s: cs43130->sclk = %d, cs43130->fs = %d, cs43130->dai_bit = %d",
+		__func__, cs43130->sclk, cs43130->fs, cs43130->dai_bit);
+	dev_dbg(codec->dev, "%s: period_size = %d, pulse_width = %d, asp_m = %d",
+		__func__, period_size, pulse_width, asp_m);
+
+	if (cs43130->dai_format) {
+		/*MSB*/
+		bick_inv = 0;
+		asp_fsd = 0;
+		asp_stp = 1;
+
+	} else {
+		/*I2S*/
+		bick_inv = 1;
+		asp_fsd = 2; /* one bick delay */
+		asp_stp = 0;
+	}
+	dev_dbg(codec->dev,
+		"%s: cs43130->dai_format = %d, bick_inv = %d, asp_fsd = %d, asp_stp = %d",
+		__func__, cs43130->dai_format, bick_inv, asp_fsd, asp_stp);
+
+	switch (cs43130->fs) {
+	case 32000:
+		asp_sprate = CS43130_ASP_SPRATE_32K;
+		break;
+	case 44100:
+		asp_sprate = CS43130_ASP_SPRATE_44_1K;
+		break;
+	case 48000:
+		asp_sprate = CS43130_ASP_SPRATE_48K;
+		break;
+	case 88200:
+		asp_sprate = CS43130_ASP_SPRATE_88_2K;
+		break;
+	case 96000:
+		asp_sprate = CS43130_ASP_SPRATE_96K;
+		break;
+	case 176400:
+		asp_sprate = CS43130_ASP_SPRATE_176_4K;
+		break;
+	case 192000:
+		asp_sprate = CS43130_ASP_SPRATE_192K;
+		break;
+	case 352800:
+		asp_sprate = CS43130_ASP_SPRATE_352_8K;
+		break;
+	case 384000:
+		asp_sprate = CS43130_ASP_SPRATE_384K;
+		break;
+	default:
+		dev_err(codec->dev, "sample rate(%d) not supported\n",
+				cs43130->fs);
+		return -EINVAL;
+	}
+	dev_dbg(codec->dev, "%s: asp_sprate = %d, cs43130->asp_size = %d",
+		__func__, asp_sprate, cs43130->asp_size);
+
+	/* ASP_SPRATE = fs*/
+	regmap_write(cs43130->regmap, CS43130_SP_SRATE, asp_sprate);
+	/*ASP_SPSIZE*/
+	regmap_update_bits(cs43130->regmap, CS43130_SP_BITSIZE,
+		CS43130_SP_BITSIZE_ASP_MASK, cs43130->asp_size);
+
+
+	/* Set up slave mode */
+	/*BICK = ASP_N/ASP_M * PLL_OUT */
+	/* ASP_N = 1 */
+	regmap_write(cs43130->regmap, CS43130_ASP_NUM_1, 1);
+	regmap_write(cs43130->regmap, CS43130_ASP_NUM_2, 0);
+
+	/*ASP_M*/
+	regmap_write(cs43130->regmap, CS43130_ASP_DENOM_1, asp_m & 0xff);
+	regmap_write(cs43130->regmap, CS43130_ASP_DENOM_2, (asp_m >> 8) & 0x3f);
+
+
+	/* H / L ratio of LRCK*/
+	regmap_write(cs43130->regmap, CS43130_ASP_LRCK_HI_TIME_1,
+	    (pulse_width-1)  & 0xff);
+	regmap_write(cs43130->regmap, CS43130_ASP_LRCK_HI_TIME_2,
+	    ((pulse_width-1) >> 8) & 0xff);
+
+	/* the period of LRCK*/
+	regmap_write(cs43130->regmap, CS43130_ASP_LRCK_PERIOD_1,
+	    (period_size-1) & 0xff);
+	regmap_write(cs43130->regmap, CS43130_ASP_LRCK_PERIOD_2,
+	    ((period_size-1) >> 8) & 0xff);
+
+	/*resolution*/
+	regmap_update_bits(cs43130->regmap, CS43130_ASP_CH_1_SZ_EN,
+		CS43130_SP_BITSIZE_ASP_MASK, cs43130->dai_bit);
+	regmap_update_bits(cs43130->regmap, CS43130_ASP_CH_2_SZ_EN,
+		CS43130_SP_BITSIZE_ASP_MASK, cs43130->dai_bit);
+
+	regmap_update_bits(cs43130->regmap, CS43130_ASP_FRAME_CONF,
+		CS43130_ASP_FSD_MASK, asp_fsd << CS43130_ASP_FSD_SHIFT);
+
+	regmap_update_bits(cs43130->regmap, CS43130_ASP_FRAME_CONF,
+		CS43130_ASP_STP_MASK, asp_stp << CS43130_ASP_STP_SHIFT);
+
+	/* set clk master/slave */
+	dev_dbg(codec->dev, "%s: cs43130->dai_mode = %d",
+	    __func__, cs43130->dai_mode);
+	regmap_update_bits(cs43130->regmap, CS43130_ASP_CLOCK_CONF,
+	    CS43130_ASP_MODE_MASK, cs43130->dai_mode << CS43130_ASP_MODE_SHIFT);
+
+	return ret;
+}
+
+static int cs43130_change_clksrc(struct snd_soc_codec *codec,
+	enum cs43130_mclk_src_sel src)
+{
+	int ret = 0;
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	regmap_update_bits(cs43130->regmap, CS43130_SYS_CLK_CTL_1,
+	    CS43130_MCLK_SRC_SEL_MASK, src << CS43130_MCLK_SRC_SEL_SHIFT);
+	regmap_update_bits(cs43130->regmap, CS43130_SYS_CLK_CTL_1,
+	    CS43130_MCLK_INT_MASK, cs43130->mclk_int << CS43130_MCLK_INT_SHIFT);
+
+	usleep_range(150, 200);
+
+	return ret;
+}
+
+static int cs43130_pcm_hw_params(struct snd_pcm_substream *substream,
+			    struct snd_pcm_hw_params *params,
+			    struct snd_soc_dai *dai)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+	unsigned int bitwidth;
+	int ret = 0;
+
+	cs43130->fs = params_rate(params);
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S8:
+		cs43130->dai_bit = CS43130_SP_BIT_SIZE_8;
+		cs43130->asp_size = CS43130_SP_BIT_SIZE_32;
+		break;
+	case SNDRV_PCM_FORMAT_S16_LE:
+		cs43130->dai_bit = CS43130_SP_BIT_SIZE_16;
+		cs43130->asp_size = CS43130_SP_BIT_SIZE_24;
+		break;
+	case SNDRV_PCM_FORMAT_S24_LE:
+		cs43130->dai_bit = CS43130_SP_BIT_SIZE_24;
+		cs43130->asp_size = CS43130_SP_BIT_SIZE_16;
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		cs43130->dai_bit = CS43130_SP_BIT_SIZE_32;
+		cs43130->asp_size = CS43130_SP_BIT_SIZE_8;
+		break;
+	default:
+		dev_err(codec->dev, "Format(%d) not supported",
+				params_format(params));
+		return -EINVAL;
+	}
+
+	bitwidth = (cs43130->dai_bit+1)*8;
+	dev_dbg(codec->dev, "(data bit)%d: (rate)%d",
+		bitwidth, cs43130->fs);
+
+	ret = cs43130_format_config(codec);
+	return ret;
+}
+
+static const DECLARE_TLV_DB_SCALE(pcm_vol_tlv, -12750, 50, 1);
+
+static const struct snd_kcontrol_new cs43130_snd_controls[] = {
+	SOC_DOUBLE_R_TLV("Master Playback Volume",
+			CS43130_PCM_VOL_A, CS43130_PCM_VOL_B, 0, 0xFF, 1,
+			pcm_vol_tlv),
+	SOC_SINGLE("Swap L/R", CS43130_PCM_PATH_CTL_2, 1, 1, 0),
+	SOC_SINGLE("Copy L/R", CS43130_PCM_PATH_CTL_2, 0, 1, 0),
+};
+
+static int cs43130_aspin_event(struct snd_soc_dapm_widget *w,
+	struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		if (cs43130->pll_bypass)
+			cs43130_change_clksrc(codec, CS43130_MCLK_SRC_XTAL);
+		else
+			cs43130_change_clksrc(codec, CS43130_MCLK_SRC_PLL);
+
+		usleep_range(10000, 10050);
+		/*ASP_3ST = 0 in master mode*/
+		if (cs43130->dai_mode)
+			regmap_update_bits(cs43130->regmap, CS43130_PAD_INT_CFG,
+						    0x01, 0x00);
+
+		regmap_update_bits(cs43130->regmap, CS43130_PWDN_CTL,
+						    CS43130_PDN_CLKOUT_MASK, 0
+						   << CS43130_PDN_CLKOUT_SHIFT);
+		break;
+	case SND_SOC_DAPM_POST_PMD:
+		break;
+	default:
+		dev_err(codec->dev, "Invalid ASPOUT event = 0x%x\n", event);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int cs43130_dac_event(struct snd_soc_dapm_widget *w,
+	struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMD:
+		cs43130_change_clksrc(codec, CS43130_MCLK_SRC_RCO);
+
+		regmap_update_bits(cs43130->regmap, CS43130_PWDN_CTL,
+						    CS43130_PDN_XTAL_MASK, 1
+						   << CS43130_PDN_XTAL_SHIFT);
+		regmap_update_bits(cs43130->regmap, CS43130_PWDN_CTL,
+						    CS43130_PDN_PLL_MASK, 1
+						   << CS43130_PDN_PLL_SHIFT);
+		break;
+	default:
+		dev_err(codec->dev, "Invalid DAC event = 0x%x\n", event);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int cs43130_hpin_event(struct snd_soc_dapm_widget *w,
+	struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMD:
+		regmap_write(cs43130->regmap, CS43130_DXD1, 0x99);
+		regmap_update_bits(cs43130->regmap, CS43130_HP_OUT_CTL_1,
+			CS43130_HP_IN_EN_MASK, 0 << CS43130_HP_IN_EN_SHIFT);
+		regmap_write(cs43130->regmap, CS43130_DXD2, 0x00);
+		regmap_write(cs43130->regmap, CS43130_DXD1, 0x00);
+		break;
+	case SND_SOC_DAPM_POST_PMU:
+		regmap_write(cs43130->regmap, CS43130_DXD1, 0x99);
+		regmap_write(cs43130->regmap, CS43130_DXD2, 0x01);
+		regmap_update_bits(cs43130->regmap, CS43130_HP_OUT_CTL_1,
+			CS43130_HP_IN_EN_MASK, 1 << CS43130_HP_IN_EN_SHIFT);
+		regmap_write(cs43130->regmap, CS43130_DXD1, 0x00);
+		break;
+	default:
+		dev_err(codec->dev, "Invalid HPIN event = 0x%x\n", event);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget cs43130_dapm_widgets[] = {
+
+	SND_SOC_DAPM_OUTPUT("HPOUTA"),
+	SND_SOC_DAPM_OUTPUT("HPOUTB"),
+
+	SND_SOC_DAPM_AIF_IN_E("ASPIN", NULL, 0, CS43130_PWDN_CTL,
+		CS43130_PDN_ASP_SHIFT, 1, cs43130_aspin_event,
+		(SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD)),
+
+	 SND_SOC_DAPM_DAC_E("HiFi DAC",
+		NULL, CS43130_PWDN_CTL, CS43130_PDN_HP_SHIFT, 1,
+		cs43130_dac_event,
+		(SND_SOC_DAPM_PRE_PMD)
+		),
+
+	SND_SOC_DAPM_LINE("Analog Playback", cs43130_hpin_event),
+};
+
+static const struct snd_soc_dapm_route cs43130_routes[] = {
+	{"ASPIN", NULL, "DAC Playback"},
+	{"HiFi DAC", NULL, "ASPIN"},
+
+	{"HPOUTA", NULL, "HiFi DAC"},
+	{"HPOUTB", NULL, "HiFi DAC"},
+	{"HPOUTA", NULL, "Analog Playback"},
+	{"HPOUTB", NULL, "Analog Playback"},
+};
+
+static const unsigned int cs43130_src_rates[] = {
+	32000, 44100, 48000, 88200, 96000, 176400, 192000, 352800, 384000
+};
+
+static const struct snd_pcm_hw_constraint_list cs43130_constraints = {
+	.count	= ARRAY_SIZE(cs43130_src_rates),
+	.list	= cs43130_src_rates,
+};
+
+static int cs43130_pcm_startup(struct snd_pcm_substream *substream,
+			       struct snd_soc_dai *dai)
+{
+	snd_pcm_hw_constraint_list(substream->runtime, 0,
+				SNDRV_PCM_HW_PARAM_RATE, &cs43130_constraints);
+	return 0;
+}
+
+static int cs43130_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFS:
+		cs43130->dai_mode = CS43130_SLAVE_MODE;
+		break;
+	case SND_SOC_DAIFMT_CBM_CFM:
+		cs43130->dai_mode = CS43130_MASTER_MODE;
+		break;
+	default:
+		dev_err(codec->dev, "unsupported i2s master mode\n");
+		return -EINVAL;
+	}
+
+	 /* interface format */
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		cs43130->dai_format = 0;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		cs43130->dai_format = 1;
+		break;
+	default:
+		dev_err(codec->dev, "unsupported audio format except I2S and MSB\n");
+		return -EINVAL;
+	}
+
+	/* BICK/LRCK pority */
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		cs43130->bick_invert = false;
+		cs43130->lrck_invert = false;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		cs43130->bick_invert = true;
+		cs43130->lrck_invert = false;
+		break;
+	case SND_SOC_DAIFMT_NB_IF:
+		cs43130->bick_invert = false;
+		cs43130->lrck_invert = true;
+		break;
+	case SND_SOC_DAIFMT_IB_IF:
+		cs43130->bick_invert = true;
+		cs43130->lrck_invert = true;
+		break;
+	default:
+		dev_err(codec->dev, "unsupported audio polarity\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+
+static int cs43130_set_mute(struct snd_soc_dai *dai, int mute)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+	int ret = 0;
+	unsigned int reg;
+	u8 mute_reg;
+
+	regmap_read(cs43130->regmap, CS43130_PCM_PATH_CTL_1, &reg);
+	mute_reg = reg & 0xfc;
+	if (mute)
+		regmap_write(cs43130->regmap, CS43130_PCM_PATH_CTL_1,
+			mute_reg | 0x03);
+	else
+		regmap_write(cs43130->regmap, CS43130_PCM_PATH_CTL_1, mute_reg);
+
+	return ret;
+}
+
+static int cs43130_set_clkdiv(struct snd_soc_dai *dai, int div_id, int div)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+
+	switch (div_id) {
+	case CS43130_AIF_BICK_RATE:
+		cs43130->bick = div;
+		break;
+	default:
+		dev_err(codec->dev,
+			"Unsupported divide value: div_id = %d", div_id);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int cs43130_set_pll(struct snd_soc_codec *codec, int pll_id, int source,
+		unsigned int freq_in, unsigned int freq_out)
+{
+	int ret = 0;
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	if (freq_in < 9600000 || freq_in > 26000000) {
+		dev_err(codec->dev,
+			"unsupported pll input reference clock:%d\n", freq_in);
+		return -EINVAL;
+	}
+
+	switch (freq_in) {
+	case 9600000:
+	case 11289600:
+	case 12000000:
+	case 12288000:
+	case 13000000:
+	case 19200000:
+	case 22579200:
+	case 24000000:
+	case 24576000:
+	case 26000000:
+		cs43130->mclk = freq_in;
+		break;
+	default:
+		dev_err(codec->dev,
+			"unsupported pll input reference clock:%d\n", freq_in);
+		return -EINVAL;
+	}
+
+	switch (freq_out) {
+	case 22579200:
+		cs43130->pll_out = freq_out;
+		cs43130->mclk_int = 1;
+		break;
+	case 24576000:
+		cs43130->pll_out = freq_out;
+		cs43130->mclk_int = 0;
+		break;
+	default:
+		dev_err(codec->dev,
+			"unsupported pll output reference clock:%d\n",
+			freq_out);
+		return -EINVAL;
+	}
+
+	ret = cs43130_pll_config(codec);
+	dev_dbg(codec->dev, "%s: cs43130->pll_bypass = %d",
+		__func__, cs43130->pll_bypass);
+	return ret;
+}
+
+static int cs43130_dai_set_sysclk(struct snd_soc_dai *codec_dai,
+		int clk_id, unsigned int freq, int dir)
+{
+	struct snd_soc_codec *codec = codec_dai->codec;
+	struct cs43130_private *cs43130 = snd_soc_codec_get_drvdata(codec);
+
+	dev_dbg(codec->dev, "%s: clk_id =  %d, freq = %d, dir = %d",
+		__func__, clk_id, freq, dir);
+	cs43130->sclk = freq;
+	return 0;
+}
+
+static const struct snd_soc_dai_ops cs43130_dai_ops = {
+	.startup	= cs43130_pcm_startup,
+	.hw_params	= cs43130_pcm_hw_params,
+	.set_sysclk	= cs43130_dai_set_sysclk,
+	.set_fmt	= cs43130_set_dai_fmt,
+	.digital_mute = cs43130_set_mute,
+	.set_clkdiv = cs43130_set_clkdiv,
+};
+
+static struct snd_soc_dai_driver cs43130_dai[] = {
+	{
+		.name = "cs43130_hifi",
+		.id = 0,
+		.playback = {
+			.stream_name = "DAC Playback",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = SNDRV_PCM_RATE_KNOT,
+			.formats = CS43130_ASP_FORMATS,
+		},
+		.ops = &cs43130_dai_ops,
+		.symmetric_rates = 1,
+	},
+	{
+		.name = "cs43130-xsp",
+		.id = 1,
+		.playback = {
+			.stream_name = "XSP Playback",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = SNDRV_PCM_RATE_KNOT,
+			.formats = CS43130_XSP_FORMATS,
+		},
+		.symmetric_rates = 1,
+	 },
+};
+
+static int cs43130_codec_set_sysclk(struct snd_soc_codec *codec,
+		int clk_id, int source, unsigned int freq, int dir)
+{
+	/* 24576000 is not supported */
+	unsigned int mclk_int_freq = 22579200;
+
+	dev_dbg(codec->dev, "%s: clk_id = %d, source = %d, freq = %d, dir = %d\n",
+		__func__, clk_id, source, freq, dir);
+	/*
+	 * freq is external mclk freq
+	 * if freq == mclk_int_freq, pll is bypassed
+	 * modify mclk_int_freq as needed for application
+	 */
+	cs43130_set_pll(codec, 0, 0, freq, mclk_int_freq);
+	return 0;
+}
+
+static irqreturn_t cs43130_irq_thread(int irq, void *data)
+{
+	struct cs43130_private *cs43130 =
+		(struct cs43130_private *)data;
+	unsigned int stickies[CS43130_NUM_INT];
+	unsigned int masks[CS43130_NUM_INT];
+	unsigned int i;
+
+	/* Read all INT status and mask reg */
+	regmap_bulk_read(cs43130->regmap, CS43130_INT_STATUS_1,
+		stickies, CS43130_NUM_INT * sizeof(unsigned int));
+	regmap_bulk_read(cs43130->regmap, CS43130_INT_MASK_1,
+		masks, CS43130_NUM_INT * sizeof(unsigned int));
+
+	for (i = 0; i < ARRAY_SIZE(stickies); i++)
+		stickies[i] = stickies[i] & (~masks[i]);
+
+	if (stickies[0] & CS43130_XTAL_RDY_INT)
+		pr_debug("%s: Crystal ready\n", __func__);
+
+	if (stickies[0] & CS43130_XTAL_ERR_INT)
+		pr_debug("%s: Crystal err\n", __func__);
+
+	if (stickies[0] & CS43130_HP_PLUG_INT)
+		pr_debug("%s: HP plugged\n", __func__);
+
+	if (stickies[0] & CS43130_HP_UNPLUG_INT)
+		pr_debug("%s: HP unplugged\n", __func__);
+
+	return IRQ_HANDLED;
+}
+static struct snd_soc_codec_driver soc_codec_dev_cs43130 = {
+	.component_driver = {
+		.controls		= cs43130_snd_controls,
+		.num_controls		= ARRAY_SIZE(cs43130_snd_controls),
+		.dapm_widgets		= cs43130_dapm_widgets,
+		.num_dapm_widgets	= ARRAY_SIZE(cs43130_dapm_widgets),
+		.dapm_routes		= cs43130_routes,
+		.num_dapm_routes	= ARRAY_SIZE(cs43130_routes),
+	},
+	.set_sysclk		= cs43130_codec_set_sysclk,
+	.set_pll		= cs43130_set_pll,
+};
+
+static const struct regmap_config cs43130_regmap = {
+	.reg_bits		= 24,
+	.pad_bits		= 8,
+	.val_bits		= 8,
+
+	.max_register		= CS43130_LASTREG,
+	.reg_defaults		= cs43130_reg_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(cs43130_reg_defaults),
+	.readable_reg		= cs43130_readable_register,
+	.precious_reg		= cs43130_precious_register,
+	.volatile_reg		= cs43130_volatile_register,
+	.cache_type		= REGCACHE_RBTREE,
+};
+
+static int cs43130_handle_device_data(
+	struct i2c_client *i2c_client, struct cs43130_private *cs43130)
+{
+	struct device_node *np = i2c_client->dev.of_node;
+	unsigned int val;
+	int ret = 0;
+
+	of_property_read_u32(np, "cirrus,xtal-ibias", &val);
+	switch (val) {
+	case 1:
+		cs43130->xtal_ibias = CS43130_XTAL_IBIAS_7_5UA;
+		break;
+	case 2:
+		cs43130->xtal_ibias = CS43130_XTAL_IBIAS_12_5UA;
+		break;
+	case 3:
+		cs43130->xtal_ibias = CS43130_XTAL_IBIAS_15UA;
+		break;
+	default:
+		dev_info(&i2c_client->dev,
+			"cirrus,xtal-ibias value or xtal unused %d",
+			val);
+	}
+	return ret;
+}
+
+static int cs43130_i2c_probe(struct i2c_client *client,
+				      const struct i2c_device_id *id)
+{
+	struct cs43130_private *cs43130;
+	int ret;
+	unsigned int devid = 0;
+	unsigned int reg;
+	int i;
+
+	cs43130 = devm_kzalloc(&client->dev, sizeof(*cs43130), GFP_KERNEL);
+	if (cs43130 == NULL)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, cs43130);
+
+	cs43130->regmap = devm_regmap_init_i2c(client, &cs43130_regmap);
+	if (IS_ERR(cs43130->regmap)) {
+		ret = PTR_ERR(cs43130->regmap);
+		return ret;
+	}
+
+	if (client->dev.of_node) {
+		ret = cs43130_handle_device_data(client, cs43130);
+		if (ret != 0)
+			return ret;
+	}
+	for (i = 0; i < ARRAY_SIZE(cs43130->supplies); i++)
+		cs43130->supplies[i].supply = cs43130_supply_names[i];
+
+	ret = devm_regulator_bulk_get(&client->dev,
+				      ARRAY_SIZE(cs43130->supplies),
+				      cs43130->supplies);
+	if (ret != 0) {
+		dev_err(&client->dev,
+			"Failed to request supplies: %d\n", ret);
+		return ret;
+	}
+	ret = regulator_bulk_enable(ARRAY_SIZE(cs43130->supplies),
+					cs43130->supplies);
+	if (ret != 0) {
+		dev_err(&client->dev,
+			"Failed to enable supplies: %d\n", ret);
+		return ret;
+	}
+
+	cs43130->reset_gpio = devm_gpiod_get_optional(&client->dev,
+		"reset", GPIOD_OUT_LOW);
+	if (IS_ERR(cs43130->reset_gpio))
+		return PTR_ERR(cs43130->reset_gpio);
+
+	usleep_range(2000, 2050);
+
+	/* initialize codec */
+	ret = regmap_read(cs43130->regmap, CS43130_DEVID_AB, &reg);
+
+	devid = (reg & 0xFF) << 12;
+	ret = regmap_read(cs43130->regmap, CS43130_DEVID_CD, &reg);
+	devid |= (reg & 0xFF) << 4;
+	ret = regmap_read(cs43130->regmap, CS43130_DEVID_E, &reg);
+	devid |= (reg & 0xF0) >> 4;
+
+	switch (devid) {
+	case CS43130_CHIP_ID:
+		break;
+	case CS4399_CHIP_ID:
+		break;
+	default:
+		dev_err(&client->dev,
+			"CS43130 Device ID (%X). Expected ID %X or %X\n",
+			devid, CS43130_CHIP_ID, CS4399_CHIP_ID);
+		ret = -ENODEV;
+		goto err;
+	}
+
+	cs43130->dev_id = devid;
+	ret = regmap_read(cs43130->regmap, CS43130_REV_ID, &reg);
+	if (ret < 0) {
+		dev_err(&client->dev, "Get Revision ID failed\n");
+		goto err;
+	}
+
+	dev_info(&client->dev,
+		 "Cirrus Logic CS43130 (%x), Revision: %02X\n", devid,
+		reg & 0xFF);
+
+	/* Enable interrupt handler */
+	ret = devm_request_threaded_irq(&client->dev,
+			client->irq,
+			NULL, cs43130_irq_thread,
+			IRQF_ONESHOT | IRQF_TRIGGER_LOW,
+			"cs43130", cs43130);
+	if (ret != 0) {
+		dev_err(&client->dev, "Failed to request IRQ: %d\n", ret);
+		return ret;
+	}
+
+	/* Unmask INT */
+	regmap_update_bits(cs43130->regmap, CS43130_INT_MASK_1,
+		CS43130_XTAL_RDY_INT | CS43130_XTAL_ERR_INT |
+		CS43130_HP_PLUG_INT | CS43130_HP_UNPLUG_INT, 0);
+
+	/* Enable HP detect */
+	regmap_update_bits(cs43130->regmap, CS43130_HP_DETECT,
+		CS43130_HP_DETECT_CTRL_MASK, CS43130_HP_DETECT_CTRL_MASK);
+
+	regmap_write(cs43130->regmap,
+		CS43130_CRYSTAL_SET, cs43130->xtal_ibias);
+	ret = snd_soc_register_codec(&client->dev,
+			&soc_codec_dev_cs43130, cs43130_dai,
+			ARRAY_SIZE(cs43130_dai));
+
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"%s: snd_soc_register_codec failed with ret = %d\n",
+			__func__, ret);
+		goto err;
+	}
+	return 0;
+err:
+	return ret;
+
+}
+
+static int cs43130_i2c_remove(struct i2c_client *client)
+{
+	snd_soc_unregister_codec(&client->dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int cs43130_runtime_suspend(struct device *dev)
+{
+	return 0;
+}
+
+static int cs43130_runtime_resume(struct device *dev)
+{
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops cs43130_runtime_pm = {
+	SET_RUNTIME_PM_OPS(cs43130_runtime_suspend, cs43130_runtime_resume,
+			   NULL)
+};
+
+static const struct of_device_id cs43130_of_match[] = {
+	{ .compatible = "cirrus,cs43130", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, cs43130_of_match);
+
+static const struct i2c_device_id cs43130_i2c_id[] = {
+	{"cs43130", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, cs43130_i2c_id);
+
+static struct i2c_driver cs43130_i2c_driver = {
+	.driver = {
+		.name		= "cs43130",
+		.of_match_table	= cs43130_of_match,
+	},
+	.id_table	= cs43130_i2c_id,
+	.probe		= cs43130_i2c_probe,
+	.remove		= cs43130_i2c_remove,
+};
+
+module_i2c_driver(cs43130_i2c_driver);
+
+MODULE_AUTHOR("Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("Cirrus Logic CS43130 ALSA SoC Codec Driver");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/cs43130.h b/sound/soc/codecs/cs43130.h
new file mode 100644
index 0000000..bceae76
--- /dev/null
+++ b/sound/soc/codecs/cs43130.h
@@ -0,0 +1,268 @@
+/*
+ * ALSA SoC CS43130 codec driver
+ *
+ * Copyright 2016 Cirrus Logic, Inc.
+ *
+ * Author: Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ */
+
+#ifndef __CS43130_H__
+#define __CS43130_H__
+
+/* CS43130 registers addresses */
+/* all reg address is shifted by a byte for control byte to be LSB */
+#define CS43130_FIRSTREG	0x010000
+#define CS43130_LASTREG		0x0F0014
+#define CS43130_CHIP_ID		0x00043130
+#define CS4399_CHIP_ID		0x00043990
+#define CS43130_DEVID_AB	0x010000         /*Device ID A & B [RO]*/
+#define CS43130_DEVID_CD	0x010001         /*Device ID C & D [RO]*/
+#define CS43130_DEVID_E		0x010002         /*Device ID E [RO]*/
+#define CS43130_FAB_ID		0x010003         /*Fab ID [RO]*/
+#define CS43130_REV_ID		0x010004         /*Revision ID [RO]*/
+#define CS43130_SUBREV_ID	0x010005         /*Subrevision ID*/
+#define CS43130_SYS_CLK_CTL_1	0x010006      /*System Clocking Ctl 1*/
+#define CS43130_SP_SRATE	0x01000B         /*Serial Port Sample Rate*/
+#define CS43130_SP_BITSIZE	0x01000C         /*Serial Port Bit Size*/
+#define CS43130_PAD_INT_CFG	0x01000D      /*Pad Interface Config*/
+#define CS43130_DXD1            0x010010        /*DXD1*/
+#define CS43130_PWDN_CTL	0x020000         /*Power Down Ctl*/
+#define CS43130_DXD2            0x020019        /*DXD2*/
+#define CS43130_CRYSTAL_SET	0x020052      /*Crystal Setting*/
+#define CS43130_PLL_SET_1	0x030001         /*PLL Setting 1*/
+#define CS43130_PLL_SET_2	0x030002         /*PLL Setting 2*/
+#define CS43130_PLL_SET_3	0x030003         /*PLL Setting 3*/
+#define CS43130_PLL_SET_4	0x030004         /*PLL Setting 4*/
+#define CS43130_PLL_SET_5	0x030005         /*PLL Setting 5*/
+#define CS43130_PLL_SET_6	0x030008         /*PLL Setting 6*/
+#define CS43130_PLL_SET_7	0x03000A         /*PLL Setting 7*/
+#define CS43130_PLL_SET_8	0x03001B         /*PLL Setting 8*/
+#define CS43130_PLL_SET_9	0x040002         /*PLL Setting 9*/
+#define CS43130_PLL_SET_10	0x040003         /*PLL Setting 10*/
+#define CS43130_CLKOUT_CTL	0x040004         /*CLKOUT Ctl*/
+#define CS43130_ASP_NUM_1	0x040010         /*ASP Numerator 1*/
+#define CS43130_ASP_NUM_2	0x040011         /*ASP Numerator 2*/
+#define CS43130_ASP_DENOM_1	0x040012      /*ASP Denominator 1*/
+#define CS43130_ASP_DENOM_2	0x040013      /*ASP Denominator 2*/
+#define CS43130_ASP_LRCK_HI_TIME_1 0x040014 /*ASP LRCK High Time 1*/
+#define CS43130_ASP_LRCK_HI_TIME_2 0x040015 /*ASP LRCK High Time 2*/
+#define CS43130_ASP_LRCK_PERIOD_1  0x040016 /*ASP LRCK Period 1*/
+#define CS43130_ASP_LRCK_PERIOD_2  0x040017 /*ASP LRCK Period 2*/
+#define CS43130_ASP_CLOCK_CONF	0x040018   /*ASP Clock Config*/
+#define CS43130_ASP_FRAME_CONF	0x040019   /*ASP Frame Config*/
+#define CS43130_XSP_NUM_1	0x040020         /*XSP Numerator 1*/
+#define CS43130_XSP_NUM_2	0x040021         /*XSP Numerator 2*/
+#define CS43130_XSP_DENOM_1	0x040022      /*XSP Denominator 1*/
+#define CS43130_XSP_DENOM_2	0x040023      /*XSP Denominator 2*/
+#define CS43130_XSP_LRCK_HI_TIME_1 0x040024 /*XSP LRCK High Time 1*/
+#define CS43130_XSP_LRCK_HI_TIME_2 0x040025 /*XSP LRCK High Time 2*/
+#define CS43130_XSP_LRCK_PERIOD_1  0x040026 /*XSP LRCK Period 1*/
+#define CS43130_XSP_LRCK_PERIOD_2  0x040027 /*XSP LRCK Period 2*/
+#define CS43130_XSP_CLOCK_CONF	0x040028   /*XSP Clock Config*/
+#define CS43130_XSP_FRAME_CONF	0x040029   /*XSP Frame Config*/
+#define CS43130_ASP_CH_1_LOC	0x050000      /*ASP Chan 1 Location*/
+#define CS43130_ASP_CH_2_LOC	0x050001      /*ASP Chan 2 Location*/
+#define CS43130_ASP_CH_1_SZ_EN	0x05000A   /*ASP Chan 1 Size, Enable*/
+#define CS43130_ASP_CH_2_SZ_EN	0x05000B   /*ASP Chan 2 Size, Enable*/
+#define CS43130_XSP_CH_1_LOC	0x060000      /*XSP Chan 1 Location*/
+#define CS43130_XSP_CH_2_LOC	0x060001      /*XSP Chan 2 Location*/
+#define CS43130_XSP_CH_1_SZ_EN	0x06000A   /*XSP Chan 1 Size, Enable*/
+#define CS43130_XSP_CH_2_SZ_EN	0x06000B   /*XSP Chan 2 Size, Enable*/
+#define CS43130_DSD_VOL_B	0x070000         /*DSD Volume B*/
+#define CS43130_DSD_VOL_A	0x070001         /*DSD Volume A*/
+#define CS43130_DSD_PATH_CTL_1	0x070002   /*DSD Proc Path Sig Ctl 1*/
+#define CS43130_DSD_INT_CFG	0x070003      /*DSD Interface Config*/
+#define CS43130_DSD_PATH_CTL_2	0x070004   /*DSD Proc Path Sig Ctl 2*/
+#define CS43130_DSD_PCM_MIX_CTL	0x070005   /*DSD and PCM Mixing Ctl*/
+#define CS43130_DSD_PATH_CTL_3	0x070006   /*DSD Proc Path Sig Ctl 3*/
+#define CS43130_HP_OUT_CTL_1	0x080000      /*HP Output Ctl 1*/
+#define CS43130_PCM_FILT_OPT	0x090000      /*PCM Filter Option*/
+#define CS43130_PCM_VOL_B	0x090001         /*PCM Volume B*/
+#define CS43130_PCM_VOL_A	0x090002         /*PCM Volume A*/
+#define CS43130_PCM_PATH_CTL_1	0x090003   /*PCM Path Signal Ctl 1*/
+#define CS43130_PCM_PATH_CTL_2	0x090004   /*PCM Path Signal Ctl 2*/
+#define CS43130_CLASS_H_CTL	0x0B0000      /*Class H Ctl*/
+#define CS43130_HP_DETECT	0x0D0000         /*HP Detect*/
+#define CS43130_HP_STATUS	0x0D0001         /*HP Status [RO]*/
+#define CS43130_HP_LOAD_1	0x0E0000         /*HP Load 1*/
+#define CS43130_HP_MEAS_LOAD_1	0x0E0003   /*HP Load Measurement 1*/
+#define CS43130_HP_MEAS_LOAD_2	0x0E0004   /*HP Load Measurement 2*/
+#define CS43130_HP_DC_STAT_1	0x0E000D      /*HP DC Load Status 0 [RO]*/
+#define CS43130_HP_DC_STAT_2	0x0E000E      /*HP DC Load Status 1 [RO]*/
+#define CS43130_HP_AC_STAT_1	0x0E0010      /*HP AC Load Status 0 [RO]*/
+#define CS43130_HP_AC_STAT_2	0x0E0011      /*HP AC Load Status 1 [RO]*/
+#define CS43130_HP_LOAD_STAT	0x0E001A      /*HP Load Status [RO]*/
+#define CS43130_INT_STATUS_1	0x0F0000      /*Interrupt Status 1*/
+#define CS43130_INT_STATUS_2	0x0F0001      /*Interrupt Status 2*/
+#define CS43130_INT_STATUS_3	0x0F0002      /*Interrupt Status 3*/
+#define CS43130_INT_STATUS_4	0x0F0003      /*Interrupt Status 4*/
+#define CS43130_INT_STATUS_5	0x0F0004      /*Interrupt Status 5*/
+#define CS43130_INT_MASK_1	0x0F0010         /*Interrupt Mask 1*/
+#define CS43130_INT_MASK_2	0x0F0011         /*Interrupt Mask 2*/
+#define CS43130_INT_MASK_3	0x0F0012         /*Interrupt Mask 3*/
+#define CS43130_INT_MASK_4	0x0F0013         /*Interrupt Mask 4*/
+#define CS43130_INT_MASK_5	0x0F0014         /*Interrupt Mask 5*/
+
+#define CS43130_MCLK_SRC_SEL_MASK		0x03
+#define CS43130_MCLK_SRC_SEL_SHIFT		0
+#define CS43130_MCLK_INT_MASK			0x04
+#define CS43130_MCLK_INT_SHIFT			2
+#define CS43130_SP_SRATE_MASK			0x0F
+#define CS43130_SP_SRATE_SHIFT			0
+#define CS43130_SP_BITSIZE_ASP_MASK		0x03
+#define CS43130_SP_BITSIZE_ASP_SHIFT	0
+#define CS43130_HP_DETECT_CTRL_SHIFT            6
+#define CS43130_HP_DETECT_CTRL_MASK     (0x03 << CS43130_HP_DETECT_CTRL_SHIFT)
+#define CS43130_HP_DETECT_INV_SHIFT             5
+#define CS43130_HP_DETECT_INV_MASK      (1 << CS43130_HP_DETECT_INV_SHIFT)
+
+/* CS43130_INT_MASK_1 */
+#define CS43130_HP_PLUG_INT_SHIFT       6
+#define CS43130_HP_PLUG_INT             (1 << CS43130_HP_PLUG_INT_SHIFT)
+#define CS43130_HP_UNPLUG_INT_SHIFT     5
+#define CS43130_HP_UNPLUG_INT           (1 << CS43130_HP_UNPLUG_INT_SHIFT)
+#define CS43130_XTAL_RDY_INT_SHIFT      4
+#define CS43130_XTAL_RDY_INT            (1 << CS43130_XTAL_RDY_INT_SHIFT)
+#define CS43130_XTAL_ERR_INT_SHIFT      3
+#define CS43130_XTAL_ERR_INT            (1 << CS43130_XTAL_ERR_INT_SHIFT)
+
+/*Reg CS43130_SP_BITSIZE*/
+#define CS43130_SP_BIT_SIZE_8			0x00
+#define CS43130_SP_BIT_SIZE_16			0x01
+#define CS43130_SP_BIT_SIZE_24			0x02
+#define CS43130_SP_BIT_SIZE_32			0x03
+
+/*PLL*/
+#define CS43130_PLL_START_MASK (0x1<<0)
+#define CS43130_PLL_MODE_MASK  0x02
+#define CS43130_PLL_MODE_SHIFT 1
+
+#define CS43130_PLL_REF_PREDIV_MASK 0x3
+
+#define CS43130_ASP_STP_MASK	0x10
+#define CS43130_ASP_STP_SHIFT	4
+#define CS43130_ASP_5050_MASK	0x08
+#define CS43130_ASP_5050_SHIFT	3
+#define CS43130_ASP_FSD_MASK	0x07
+#define CS43130_ASP_FSD_SHIFT	0
+
+#define CS43130_ASP_MODE_MASK	0x10
+#define CS43130_ASP_MODE_SHIFT	4
+#define CS43130_ASP_SCPOL_OUT_MASK	0x08
+#define CS43130_ASP_SCPOL_OUT_SHIFT	3
+#define CS43130_ASP_SCPOL_IN_MASK	0x04
+#define CS43130_ASP_SCPOL_IN_SHIFT	2
+#define CS43130_ASP_LCPOL_OUT_MASK	0x02
+#define CS43130_ASP_LCPOL_OUT_SHIFT	1
+#define CS43130_ASP_LCPOL_IN_MASK	0x01
+#define CS43130_ASP_LCPOL_IN_SHIFT	0
+
+/*Reg CS43130_PWDN_CTL*/
+#define CS43130_PDN_XSP_MASK	0x80
+#define CS43130_PDN_XSP_SHIFT	7
+#define CS43130_PDN_ASP_MASK	0x40
+#define CS43130_PDN_ASP_SHIFT	6
+#define CS43130_PDN_DSPIF_MASK	0x20
+#define CS43130_PDN_DSDIF_SHIFT	5
+#define CS43130_PDN_HP_MASK	0x10
+#define CS43130_PDN_HP_SHIFT	4
+#define CS43130_PDN_XTAL_MASK	0x08
+#define CS43130_PDN_XTAL_SHIFT	3
+#define CS43130_PDN_PLL_MASK	0x04
+#define CS43130_PDN_PLL_SHIFT	2
+#define CS43130_PDN_CLKOUT_MASK	0x02
+#define CS43130_PDN_CLKOUT_SHIFT	1
+
+#define CS43130_7_0_MASK		0xFF
+#define CS43130_15_8_MASK		0xFF00
+#define CS43130_23_16_MASK		0xFF0000
+
+/* Reg CS43130_HP_OUT_CTL_1 */
+#define CS43130_HP_IN_EN_SHIFT		3
+#define CS43130_HP_IN_EN_MASK		0x08
+
+#define CS43130_ASP_FORMATS (SNDRV_PCM_FMTBIT_S8  | \
+			SNDRV_PCM_FMTBIT_S16_LE | \
+			SNDRV_PCM_FMTBIT_S24_LE | \
+			SNDRV_PCM_FMTBIT_S32_LE)
+
+#define CS43130_XSP_FORMATS (SNDRV_PCM_FMTBIT_S24_LE | \
+			SNDRV_PCM_FMTBIT_S32_LE)
+
+enum cs43130_asp_rate {
+	CS43130_ASP_SPRATE_32K = 0,
+	CS43130_ASP_SPRATE_44_1K,
+	CS43130_ASP_SPRATE_48K,
+	CS43130_ASP_SPRATE_88_2K,
+	CS43130_ASP_SPRATE_96K,
+	CS43130_ASP_SPRATE_176_4K,
+	CS43130_ASP_SPRATE_192K,
+	CS43130_ASP_SPRATE_352_8K,
+	CS43130_ASP_SPRATE_384K,
+};
+
+enum cs43130_mclk_src_sel {
+	CS43130_MCLK_SRC_XTAL = 0,
+	CS43130_MCLK_SRC_PLL,
+	CS43130_MCLK_SRC_RCO
+};
+
+enum cs43130_mode {
+	CS43130_SLAVE_MODE = 0,
+	CS43130_MASTER_MODE
+};
+
+enum cs43130_xtal_ibias {
+	CS43130_XTAL_IBIAS_15UA = 2,
+	CS43130_XTAL_IBIAS_12_5UA = 4,
+	CS43130_XTAL_IBIAS_7_5UA = 6,
+};
+
+#define CS43130_AIF_BICK_RATE 1
+#define CS43130_SYSCLK_MCLK 1
+#define CS43130_NUM_SUPPLIES 5
+static const char *const cs43130_supply_names[CS43130_NUM_SUPPLIES] = {
+	"VA",
+	"VP",
+	"VCP",
+	"VD",
+	"VL",
+};
+
+#define CS43130_NUM_INT 5       /* number of interrupt status reg */
+
+struct	cs43130_private {
+	struct snd_soc_codec		*codec;
+	struct regmap			*regmap;
+	struct regulator_bulk_data supplies[CS43130_NUM_SUPPLIES];
+	/* codec device ID */
+	unsigned int dev_id;
+	int				mclk;
+	int				sclk;
+	int xtal_ibias;
+
+	bool pll_bypass;
+	int pll_out;
+	int mclk_int;
+	int dai_format;
+	int dai_mode;
+	int dai_bit;
+	int asp_size;
+	int fs;
+	bool bick_invert;
+	bool lrck_invert;
+	int bick;
+	struct gpio_desc *reset_gpio;
+};
+
+#endif	/* __CS43130_H__ */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 2/2] ASoC: cs43130: Add devicetree bindings for CS43130
From: Li Xu @ 2016-12-07 20:17 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	perex-/Fr2/VpizcU, tiwai-IBi9RG/b67k,
	brian.austin-jGc1dHjMKG3QT0dZR+AlfA,
	Paul.Handrigan-jGc1dHjMKG3QT0dZR+AlfA, Li Xu
In-Reply-To: <1481141848-6695-1-git-send-email-li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>

Add devicetree bindings documentation file for Cirrus
Logic CS43130 codec.

Signed-off-by: Li Xu <li.xu-jGc1dHjMKG3QT0dZR+AlfA@public.gmane.org>
---
 .../devicetree/bindings/sound/cs43130.txt          | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/cs43130.txt

diff --git a/Documentation/devicetree/bindings/sound/cs43130.txt b/Documentation/devicetree/bindings/sound/cs43130.txt
new file mode 100644
index 0000000..9a2a22a
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/cs43130.txt
@@ -0,0 +1,41 @@
+CS43130 DAC
+
+Required properties:
+
+  - compatible : "cirrus,cs43130"
+
+  - reg : the I2C address of the device for I2C
+
+  - VA-supply, VP-supply, VL-supply, VCP-supply, VD-supply:
+	power supplies for the device, as covered in
+	Documentation/devicetree/bindings/regulator/regulator.txt.
+
+
+Optional properties:
+
+  - reset-gpios : gpio used to reset the Device
+
+  - cirrus,xtal-ibias:
+   When external MCLK is generated by external crystal
+   oscillator, CS43130 can be used to provide bias current
+   for external crystal.  Amount of bias current sent is
+   set as:
+   1 = 7.5uA
+   2 = 12.5uA
+   3 = 15uA
+
+Example:
+
+cs43130: cs43130@30 {
+	compatible = "cirrus,cs43130";
+	reg = <0x30>;
+	reset-gpios = <&axi_gpio 54 0>;
+   VA-supply = <&dummy_vreg>;
+   VP-supply = <&dummy_vreg>;
+   VL-supply = <&dummy_vreg>;
+   VCP-supply = <&dummy_vreg>;
+   VD-supply = <&dummy_vreg>;
+   cirrus,xtal-ibias = <2>;
+   interrupt-parent = <&gpio0>;
+   interrupts = <55 8>;
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH] dts: sun8i-h3: correct UART3 pin definitions
From: Olof Johansson @ 2016-12-07 20:40 UTC (permalink / raw)
  To: jorik-U9/BOH3cVv3CLqq/8VZgpA
  Cc: arm-DgEjT+Ai2ygdnm+yROfE0A,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161206142710.6450-1-jorik-U9/BOH3cVv3CLqq/8VZgpA@public.gmane.org>

On Tue, Dec 06, 2016 at 03:27:10PM +0100, jorik-U9/BOH3cVv3CLqq/8VZgpA@public.gmane.org wrote:
> From: Jorik Jonker <jorik-U9/BOH3cVv3CLqq/8VZgpA@public.gmane.org>
> 
> In a previous commit, I made a copy/paste error in the pinmux
> definitions of UART3: PG{13,14} instead of PA{13,14}. This commit takes
> care of that. I have tested this commit on Orange Pi PC and Orange Pi
> Plus, and it works for these boards.
> 
> Fixes: e3d11d3c45c5 ("dts: sun8i-h3: add pinmux definitions for
> UART2-3")
> 
> Signed-off-by: Jorik Jonker <jorik-U9/BOH3cVv3CLqq/8VZgpA@public.gmane.org>

Applied to fixes. Thanks.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx7d: fix LCDIF clock assignment
From: Olof Johansson @ 2016-12-07 20:53 UTC (permalink / raw)
  To: Shawn Guo
  Cc: Arnd Bergmann, Sascha Hauer, Stefan Agner, Mark Rutland,
	devicetree, linux-kernel, robh+dt, Peter Chen, Fabio Estevam,
	Liu Ying, linux-arm-kernel, Fabio Estevam
In-Reply-To: <20161205020123.GA2066@dragon>

On Mon, Dec 05, 2016 at 10:01:24AM +0800, Shawn Guo wrote:
> Hi Arnd, Olof,
> 
> On Sun, Dec 04, 2016 at 05:26:58PM -0800, Stefan Agner wrote:
> > Hi Shawn
> > 
> > On 2016-11-23 15:02, Fabio Estevam wrote:
> > > On Tue, Nov 22, 2016 at 10:42 PM, Stefan Agner <stefan@agner.ch> wrote:
> > >> The eLCDIF IP of the i.MX 7 SoC knows multiple clocks and lists them
> > >> separately:
> > >>
> > >> Clock      Clock Root              Description
> > >> apb_clk    MAIN_AXI_CLK_ROOT       AXI clock
> > >> pix_clk    LCDIF_PIXEL_CLK_ROOT    Pixel clock
> > >> ipg_clk_s  MAIN_AXI_CLK_ROOT       Peripheral access clock
> > >>
> > >> All of them are switched by a single gate, which is part of the
> > >> IMX7D_LCDIF_PIXEL_ROOT_CLK clock. Hence using that clock also for
> > >> the AXI bus clock (clock-name "axi") makes sure the gate gets
> > >> enabled when accessing registers.
> > >>
> > >> There seem to be no separate AXI display clock, and the clock is
> > >> optional. Hence remove the dummy clock.
> > >>
> > >> This fixes kernel freezes when starting the X-Server (which
> > >> disables/re-enables the display controller).
> > >>
> > >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> > > 
> > > Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> > 
> > Since this fixes a kernel freeze, is there a chance to get this still in
> > 4.9?
> 
> Since we get one more week to the final 4.9, is it possible for you to
> send this fix for 4.9 inclusion?  Thanks.
> 
> For the patch,
> 
> Acked-by: Shawn Guo <shawnguo@kernel.org>

Applied, with the fixes line. In the future, please email arm@kernel.org too,
it's easier to make sure we don't miss it that way.


-Olof

^ permalink raw reply

* [PATCH v5 0/3] Altera Cyclone Passive Serial SPI FPGA Manager
From: Joshua Clayton @ 2016-12-07 21:04 UTC (permalink / raw)
  To: Alan Tull, Moritz Fischer, Mark Rutland, Russell King
  Cc: devicetree, Joshua Clayton, linux-kernel, Rob Herring,
	Anatolij Gustschin, linux-arm-kernel

This series adds an FPGA manager for Altera cyclone FPGAs
that can program them using an spi port and a couple of gpios, using
Alteras passive serial protocol.

Still need an ACK from Russell King on the ARM specific portion of 
patch 1. Any comment/criticism on the addition of bitrev8x4() to bitrev.h
would be welcome as well.

Changes from v4:
- Added the needed return statement to __arch_bitrev8x4()
- Added Rob Herrings ACK for and fix a typo in the commit log of patch 2

Changes from v3:
- Fixed up the state() function to return the state of the status pin
  reqested by Alan Tull
- Switched the pin to ACTIVE_LOW and coresponding logic level, and updated
  the corresponding documentation. Thanks Rob Herring for pointing out my
  mistake.
- Per Rob Herring, switched from "gpio" to "gpios" in dts

Changes from v2:
- Merged patch 3 and 4 as suggested in review by Moritz Fischer
- Changed FPGA_MIN_DELAY from 250 to 50 ms is the time advertized by
  Altera. This now works, as we don't assume it is done

Changes from v1:
- Changed the name from cyclone-spi-fpga-mgr to cyclone-ps-spi-fpga-mgr
  This name change was requested by Alan Tull, to be specific about which
  programming method is being employed on the fpga.
- Changed the name of the reset-gpio to config-gpio to closer match the
  way the pins are described in the Altera manual
- Moved MODULE_LICENCE, _AUTHOR, and _DESCRIPTION to the bottom

- Added a bitrev8x4() function to the bitrev headers and implemented ARM
 const, runtime, and ARM specific faster versions (This may end up
 needing to be a standalone patch)

- Moved the bitswapping into cyclonespi_write(), as requested.
  This falls short of my desired generic lsb first spi support, but is a step
  in that direction.

- Fixed whitespace problems introduced during refactoring

- Replaced magic number for initial delay with a descriptive macro
- Poll the fpga to see when it is ready rather than a fixed 1 ms sleep

Joshua Clayton (3):
  lib: add bitrev8x4()
  doc: dt: add cyclone-ps-spi binding document
  fpga manager: Add cyclone-ps-spi driver for Altera FPGAs

 .../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt      |  25 +++
 arch/arm/include/asm/bitrev.h                      |   6 +
 drivers/fpga/Kconfig                               |   7 +
 drivers/fpga/Makefile                              |   1 +
 drivers/fpga/cyclone-ps-spi.c                      | 181 +++++++++++++++++++++
 include/linux/bitrev.h                             |  26 +++
 6 files changed, 246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
 create mode 100644 drivers/fpga/cyclone-ps-spi.c

-- 
2.9.3

^ permalink raw reply

* [PATCH v5 1/3] lib: add bitrev8x4()
From: Joshua Clayton @ 2016-12-07 21:04 UTC (permalink / raw)
  To: Alan Tull, Moritz Fischer, Mark Rutland, Russell King
  Cc: devicetree, Joshua Clayton, linux-kernel, Rob Herring,
	Anatolij Gustschin, linux-arm-kernel
In-Reply-To: <cover.1481139171.git.stillcompiling@gmail.com>

Add a function to reverse bytes within a 32 bit word.
This function is more efficient than using the 8 bit version when
iterating over an array

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
 arch/arm/include/asm/bitrev.h |  6 ++++++
 include/linux/bitrev.h        | 26 ++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
index ec291c3..9482f78 100644
--- a/arch/arm/include/asm/bitrev.h
+++ b/arch/arm/include/asm/bitrev.h
@@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
 	return __arch_bitrev32((u32)x) >> 24;
 }
 
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+	__asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+	return x;
+}
+
 #endif
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index fb790b8..b1cfa1a 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -9,6 +9,7 @@
 #define __bitrev32 __arch_bitrev32
 #define __bitrev16 __arch_bitrev16
 #define __bitrev8 __arch_bitrev8
+#define __bitrev8x4 __arch_bitrev8x4
 
 #else
 extern u8 const byte_rev_table[256];
@@ -27,6 +28,14 @@ static inline u32 __bitrev32(u32 x)
 	return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
 }
 
+static inline u32 __bitrev8x4(u32 x)
+{
+	return(__bitrev8(x & 0xff) |
+	       (__bitrev8((x >> 8)  & 0xff) << 8) |
+	       (__bitrev8((x >> 16)  & 0xff) << 16) |
+	       (__bitrev8((x >> 24)  & 0xff) << 24));
+}
+
 #endif /* CONFIG_HAVE_ARCH_BITREVERSE */
 
 #define __constant_bitrev32(x)	\
@@ -50,6 +59,15 @@ static inline u32 __bitrev32(u32 x)
 	__x;								\
 })
 
+#define __constant_bitrev8x4(x) \
+({			\
+	u32 __x = x;	\
+	__x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4);	\
+	__x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2);	\
+	__x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1);	\
+	__x;								\
+})
+
 #define __constant_bitrev8(x)	\
 ({					\
 	u8 __x = x;			\
@@ -75,6 +93,14 @@ static inline u32 __bitrev32(u32 x)
 	__bitrev16(__x);				\
  })
 
+#define bitrev8x4(x) \
+({			\
+	u32 __x = x;	\
+	__builtin_constant_p(__x) ?	\
+	__constant_bitrev8x4(__x) :			\
+	__bitrev8x4(__x);				\
+})
+
 #define bitrev8(x) \
 ({			\
 	u8 __x = x;	\
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 2/3] doc: dt: add cyclone-ps-spi binding document
From: Joshua Clayton @ 2016-12-07 21:04 UTC (permalink / raw)
  To: Alan Tull, Moritz Fischer, Mark Rutland, Russell King
  Cc: devicetree, Joshua Clayton, linux-kernel, Rob Herring,
	Anatolij Gustschin, linux-arm-kernel
In-Reply-To: <cover.1481139171.git.stillcompiling@gmail.com>

Describe a cyclone-ps-spi devicetree entry, required features

Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt      | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt

diff --git a/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
new file mode 100644
index 0000000..3f515c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
@@ -0,0 +1,25 @@
+Altera Cyclone Passive Serial SPI FPGA Manager
+
+Altera Cyclone FPGAs support a method of loading the bitstream over what is
+referred to as "passive serial".
+The passive serial link is not technically spi, and might require extra
+circuits in order to play nicely with other spi slaves on the same bus.
+
+See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
+
+Required properties:
+- compatible  : should contain "altr,cyclone-ps-spi-fpga-mgr"
+- reg         : spi slave id of the fpga
+- config-gpios : config pin (referred to as nCONFIG in the cyclone manual)
+- status-gpios : status pin (referred to as nSTATUS in the cyclone manual)
+
+both gpios pins are normally active low open drain.
+
+Example:
+	fpga_spi: evi-fpga-spi@0 {
+		compatible = "altr,cyclone-ps-spi-fpga-mgr";
+		spi-max-frequency = <20000000>;
+		reg = <0>;
+		config-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+		status-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+	};
-- 
2.9.3

^ permalink raw reply related

* [PATCH v5 3/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: Joshua Clayton @ 2016-12-07 21:04 UTC (permalink / raw)
  To: Alan Tull, Moritz Fischer, Mark Rutland, Russell King
  Cc: Rob Herring, Anatolij Gustschin, Joshua Clayton,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <cover.1481139171.git.stillcompiling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

cyclone-ps-spi loads FPGA firmware over spi, using the "passive serial"
interface on Altera Cyclone FPGAS.

This is one of the simpler ways to set up an FPGA at runtime.
The signal interface is close to unidirectional spi with lsb first.

Signed-off-by: Joshua Clayton <stillcompiling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/fpga/Kconfig          |   7 ++
 drivers/fpga/Makefile         |   1 +
 drivers/fpga/cyclone-ps-spi.c | 181 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 189 insertions(+)
 create mode 100644 drivers/fpga/cyclone-ps-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index cd84934..2462707 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -13,6 +13,13 @@ config FPGA
 
 if FPGA
 
+config FPGA_MGR_CYCLONE_PS_SPI
+	tristate "Altera Cyclone FPGA Passive Serial over SPI"
+	depends on SPI
+	help
+	  FPGA manager driver support for Altera Cyclone using the
+	  passive serial interface over SPI
+
 config FPGA_MGR_SOCFPGA
 	tristate "Altera SOCFPGA FPGA Manager"
 	depends on ARCH_SOCFPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8d83fc6..8f93930 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -6,5 +6,6 @@
 obj-$(CONFIG_FPGA)			+= fpga-mgr.o
 
 # FPGA Manager Drivers
+obj-$(CONFIG_FPGA_MGR_CYCLONE_PS_SPI)	+= cyclone-ps-spi.o
 obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
new file mode 100644
index 0000000..82a754a
--- /dev/null
+++ b/drivers/fpga/cyclone-ps-spi.c
@@ -0,0 +1,181 @@
+/**
+ * Copyright (c) 2015 United Western Technologies, Corporation
+ *
+ * Joshua Clayton <stillcompiling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * Manage Altera fpga firmware that is loaded over spi.
+ * Firmware must be in binary "rbf" format.
+ * Works on Cyclone V. Should work on cyclone series.
+ * May work on other Altera fpgas.
+ *
+ */
+
+#include <linux/bitrev.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/sizes.h>
+
+#define FPGA_RESET_TIME		50   /* time in usecs to trigger FPGA config */
+#define FPGA_MIN_DELAY		50   /* min usecs to wait for config status */
+#define FPGA_MAX_DELAY		1000 /* max usecs to wait for config status */
+
+struct cyclonespi_conf {
+	struct gpio_desc *config;
+	struct gpio_desc *status;
+	struct spi_device *spi;
+};
+
+static const struct of_device_id of_ef_match[] = {
+	{ .compatible = "altr,cyclone-ps-spi-fpga-mgr", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_ef_match);
+
+static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+	if (gpiod_get_value(conf->status))
+		return FPGA_MGR_STATE_RESET;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
+				 const char *buf, size_t count)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+	int i;
+
+	if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
+		return -EINVAL;
+	}
+
+	gpiod_set_value(conf->config, 1);
+	usleep_range(FPGA_RESET_TIME, FPGA_RESET_TIME + 20);
+	if (!gpiod_get_value(conf->status)) {
+		dev_err(&mgr->dev, "Status pin should be low.\n");
+		return -EIO;
+	}
+
+	gpiod_set_value(conf->config, 0);
+	for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
+		usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+		if (!gpiod_get_value(conf->status))
+			return 0;
+	}
+
+	dev_err(&mgr->dev, "Status pin not ready.\n");
+	return -EIO;
+}
+
+static void rev_buf(void *buf, size_t len)
+{
+	u32 *fw32 = (u32 *)buf;
+	const u32 *fw_end = (u32 *)(buf + len);
+
+	/* set buffer to lsb first */
+	while (fw32 < fw_end) {
+		*fw32 = bitrev8x4(*fw32);
+		fw32++;
+	}
+}
+
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+			    size_t count)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+	const char *fw_data = buf;
+	const char *fw_data_end = fw_data + count;
+
+	while (fw_data < fw_data_end) {
+		int ret;
+		size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+		rev_buf((void *)fw_data, stride);
+		ret = spi_write(conf->spi, fw_data, stride);
+		if (ret) {
+			dev_err(&mgr->dev, "spi error in firmware write: %d\n",
+				ret);
+			return ret;
+		}
+		fw_data += stride;
+	}
+
+	return 0;
+}
+
+static int cyclonespi_write_complete(struct fpga_manager *mgr, u32 flags)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+	if (gpiod_get_value(conf->status)) {
+		dev_err(&mgr->dev, "Error during configuration.\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static const struct fpga_manager_ops cyclonespi_ops = {
+	.state = cyclonespi_state,
+	.write_init = cyclonespi_write_init,
+	.write = cyclonespi_write,
+	.write_complete = cyclonespi_write_complete,
+};
+
+static int cyclonespi_probe(struct spi_device *spi)
+{
+	struct cyclonespi_conf *conf = devm_kzalloc(&spi->dev, sizeof(*conf),
+						GFP_KERNEL);
+
+	if (!conf)
+		return -ENOMEM;
+
+	conf->spi = spi;
+	conf->config = devm_gpiod_get(&spi->dev, "config", GPIOD_OUT_HIGH);
+	if (IS_ERR(conf->config)) {
+		dev_err(&spi->dev, "Failed to get config gpio: %ld\n",
+			PTR_ERR(conf->config));
+		return PTR_ERR(conf->config);
+	}
+
+	conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
+	if (IS_ERR(conf->status)) {
+		dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
+			PTR_ERR(conf->status));
+		return PTR_ERR(conf->status);
+	}
+
+	return fpga_mgr_register(&spi->dev,
+				 "Altera Cyclone PS SPI FPGA Manager",
+				 &cyclonespi_ops, conf);
+}
+
+static int cyclonespi_remove(struct spi_device *spi)
+{
+	fpga_mgr_unregister(&spi->dev);
+
+	return 0;
+}
+
+static struct spi_driver cyclonespi_driver = {
+	.driver = {
+		.name   = "cyclone-ps-spi",
+		.owner  = THIS_MODULE,
+		.of_match_table = of_match_ptr(of_ef_match),
+	},
+	.probe  = cyclonespi_probe,
+	.remove = cyclonespi_remove,
+};
+
+module_spi_driver(cyclonespi_driver)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joshua Clayton <stillcompiling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("Module to load Altera FPGA firmware over spi");
-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v5 3/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: Anatolij Gustschin @ 2016-12-07 21:21 UTC (permalink / raw)
  To: Joshua Clayton
  Cc: Mark Rutland, Moritz Fischer, devicetree, Alan Tull, Russell King,
	linux-kernel, Rob Herring, linux-arm-kernel
In-Reply-To: <1778a82d1989d13919b24e47fa09eeb56b2cb8e5.1481139171.git.stillcompiling@gmail.com>

On Wed,  7 Dec 2016 13:04:40 -0800
Joshua Clayton stillcompiling@gmail.com wrote:
...
>+static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
>+				 const char *buf, size_t count)

there is a minor API change in linux-next [1]. struct fpga_image_info *
is passed instead of u32 flags. I assume this will be merged soon, so
please prepare this driver accordingly.

...
>+static int cyclonespi_write_complete(struct fpga_manager *mgr, u32 flags)

same here.

[1] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/fpga?id=1df2865f8dd9d56cb76aa7aa1298921e7bece2af

--
Anatolij

^ permalink raw reply

* Re: [PATCH v5 3/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: Joshua Clayton @ 2016-12-07 21:29 UTC (permalink / raw)
  To: Anatolij Gustschin
  Cc: Alan Tull, Moritz Fischer, Mark Rutland, Russell King,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161207222127.23f6b8b4@crub>

Anatolij,
Thanks for the quick look and response.
I guess I should have rebased before submitting.
I just realized I also forgot to send this patch series to
the newly minted FPGA Manager mailing list. V6 coming soon :)

On 12/07/2016 01:21 PM, Anatolij Gustschin wrote:
> On Wed,  7 Dec 2016 13:04:40 -0800
> Joshua Clayton stillcompiling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> ...
>> +static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
>> +				 const char *buf, size_t count)
> there is a minor API change in linux-next [1]. struct fpga_image_info *
> is passed instead of u32 flags. I assume this will be merged soon, so
> please prepare this driver accordingly.
>
> ...
Ah. Thanks.

>> +static int cyclonespi_write_complete(struct fpga_manager *mgr, u32 flags)
> same here.
OK.
>
> [1] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/fpga?id=1df2865f8dd9d56cb76aa7aa1298921e7bece2af
>
> --
> Anatolij
Joshua
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 3/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: Anatolij Gustschin @ 2016-12-07 21:33 UTC (permalink / raw)
  To: Joshua Clayton
  Cc: Alan Tull, Moritz Fischer, Mark Rutland, Russell King,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <6e33619c-e8a4-e372-04ab-d03c379cbfa0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Wed, 7 Dec 2016 13:29:29 -0800
Joshua Clayton stillcompiling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:

>Anatolij,
>Thanks for the quick look and response.
>I guess I should have rebased before submitting.

yes, Makefile and Kconfig changes need rebasing, I think.

>I just realized I also forgot to send this patch series to
>the newly minted FPGA Manager mailing list. V6 coming soon :)

okay, thanks.

--
Anatolij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Sakari Ailus @ 2016-12-07 22:33 UTC (permalink / raw)
  To: Ramiro Oliveira
  Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
	slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
	robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
	pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
	sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
	CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <bb5a2ae3078a977eb52aec0ffa3a0a0de558e6ad.1480958609.git.roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

Hi Ramiro,

Thank you for the patch.

On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
> Add device tree documentation.
> 
> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> ---
>  .../devicetree/bindings/media/i2c/ov5647.txt          | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> new file mode 100644
> index 0000000..4c91b3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> @@ -0,0 +1,19 @@
> +Omnivision OV5647 raw image sensor
> +---------------------------------
> +
> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
> +and CCI (I2C compatible) control bus.
> +
> +Required properties:
> +
> +- compatible	: "ovti,ov5647";
> +- reg		: I2C slave address of the sensor;
> +
> +The common video interfaces bindings (see video-interfaces.txt) should be
> +used to specify link to the image data receiver. The OV5647 device
> +node should contain one 'port' child node with an 'endpoint' subnode.
> +
> +Following properties are valid for the endpoint node:
> +
> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
> +  video-interfaces.txt.  The sensor supports only two data lanes.

Doesn't this sensor require a external clock, a reset GPIO and / or a
regulator or a few? Do you need data-lanes, unless you can change the order
or the number?

An example DT snippet wouldn't hurt.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] ARM: dts: lpc32xx: add pwm-cells to base dts file
From: Vladimir Zapolskiy @ 2016-12-07 22:50 UTC (permalink / raw)
  To: Sylvain Lemieux, robh+dt; +Cc: devicetree, linux-arm-kernel
In-Reply-To: <1481139047-13166-1-git-send-email-slemieux.tyco@gmail.com>

Hi Sylvain,

On 12/07/2016 09:30 PM, Sylvain Lemieux wrote:
> From: Sylvain Lemieux <slemieux@tycoint.com>
> 
> There is no need to define the "pwm-cells" in the board
> specific dts file; move the entry to the base dts file.
> 
> Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
> ---

thank you for the change,

Acked-by: Vladimir Zapolskiy <vz@mleia.com>

--
With best wishes,
Vladimir

^ permalink raw reply

* Re: [PATCH pci/next v3 0/3] PCI: rcar, rcar-gen2: Bindings cleanups
From: Bjorn Helgaas @ 2016-12-07 22:57 UTC (permalink / raw)
  To: Simon Horman
  Cc: Bjorn Helgaas, Phil Edworthy, Magnus Damm, linux-pci,
	linux-renesas-soc, Rob Herring, devicetree
In-Reply-To: <1481039491-30364-1-git-send-email-horms+renesas@verge.net.au>

On Tue, Dec 06, 2016 at 04:51:28PM +0100, Simon Horman wrote:
> Hi,
> 
> this short series makes some bindings cleanups to the Renesas PCI drivers.
> 
> Changes v2->v3:
> * Reworded changelogs to indicate that re-ordering struct of_device_id
>   entries does not effect run-time behaviour
> * Corrected compile error due to typo in symbol name
> Changes v1->v2:
> * re-order struct of_device_id entries
> 
> Simon Horman (3):
>   PCI: rcar-gen2: Use gen2 fallback compatibility last
>   PCI: rcar: Use gen2 fallback compatibility last
>   PCI: rcar: Add gen3 fallback compatibility string for pcie-rcar
> 
>  Documentation/devicetree/bindings/pci/rcar-pci.txt | 1 +
>  drivers/pci/host/pci-rcar-gen2.c                   | 2 +-
>  drivers/pci/host/pcie-rcar.c                       | 5 +++--
>  3 files changed, 5 insertions(+), 3 deletions(-)

Applied to pci/host-rcar for v4.10, thanks!

^ permalink raw reply

* Re: [PATCH v5 2/2] Add support for OV5647 sensor
From: Sakari Ailus @ 2016-12-07 23:01 UTC (permalink / raw)
  To: Ramiro Oliveira
  Cc: mchehab, linux-kernel, linux-media, robh+dt, devicetree, davem,
	gregkh, geert+renesas, akpm, linux, hverkuil, dheitmueller,
	slongerbeam, lars, robert.jarzmik, pavel, pali.rohar,
	sakari.ailus, mark.rutland, CARLOS.PALMINHA
In-Reply-To: <3e6262362f9961d2cf861353f32dbcd5bcc5a879.1480958609.git.roliveir@synopsys.com>

Hi Ramiro,

On Mon, Dec 05, 2016 at 05:36:34PM +0000, Ramiro Oliveira wrote:
> Add support for OV5647 sensor.
> 
> Modes supported:
>  - 640x480 RAW 8
> 
> Signed-off-by: Ramiro Oliveira <roliveir@synopsys.com>
> ---
>  MAINTAINERS                |   7 +
>  drivers/media/i2c/Kconfig  |  12 +
>  drivers/media/i2c/Makefile |   1 +
>  drivers/media/i2c/ov5647.c | 866 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 886 insertions(+)
>  create mode 100644 drivers/media/i2c/ov5647.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 52cc077..72e828a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8923,6 +8923,13 @@ M:	Harald Welte <laforge@gnumonks.org>
>  S:	Maintained
>  F:	drivers/char/pcmcia/cm4040_cs.*
>  
> +OMNIVISION OV5647 SENSOR DRIVER
> +M:	Ramiro Oliveira <roliveir@synopsys.com>
> +L:	linux-media@vger.kernel.org
> +T:	git git://linuxtv.org/media_tree.git
> +S:	Maintained
> +F:	drivers/media/i2c/ov5647.c
> +
>  OMNIVISION OV7670 SENSOR DRIVER
>  M:	Jonathan Corbet <corbet@lwn.net>
>  L:	linux-media@vger.kernel.org
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index b31fa6f..c1b78e5 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -531,6 +531,18 @@ config VIDEO_OV2659
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ov2659.
>  
> +config VIDEO_OV5647
> +	tristate "OmniVision OV5647 sensor support"
> +	depends on OF

How does this driver depend on OF, other than matching the compatible
string?

> +	depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> +	depends on MEDIA_CAMERA_SUPPORT
> +	---help---
> +	  This is a Video4Linux2 sensor-level driver for the OmniVision
> +	  OV5647 camera.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ov5647.
> +
>  config VIDEO_OV7640
>  	tristate "OmniVision OV7640 sensor support"
>  	depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 92773b2..0d9014c 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)	+= ml86v7667.o
>  obj-$(CONFIG_VIDEO_OV2659)	+= ov2659.o
>  obj-$(CONFIG_VIDEO_TC358743)	+= tc358743.o
> +obj-$(CONFIG_VIDEO_OV5647)	+= ov5647.o
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> new file mode 100644
> index 0000000..2aae806
> --- /dev/null
> +++ b/drivers/media/i2c/ov5647.c
> @@ -0,0 +1,866 @@
> +/*
> + * A V4L2 driver for OmniVision OV5647 cameras.
> + *
> + * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
> + * Copyright (C) 2011 Sylwester Nawrocki <s.nawrocki@samsung.com>
> + *
> + * Based on Omnivision OV7670 Camera Driver
> + * Copyright (C) 2006-7 Jonathan Corbet <corbet@lwn.net>
> + *
> + * Copyright (C) 2016, Synopsys, Inc.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/videodev2.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-mediabus.h>
> +#include <media/v4l2-image-sizes.h>
> +#include <media/v4l2-of.h>
> +#include <linux/io.h>

Alphabetical order, please.

> +
> +#define SENSOR_NAME "ov5647"
> +
> +#define OV5647_SW_RESET		0x1003
> +#define OV5647_REG_CHIPID_H	0x300A
> +#define OV5647_REG_CHIPID_L	0x300B
> +
> +#define REG_TERM 0xfffe
> +#define VAL_TERM 0xfe
> +#define REG_DLY  0xffff
> +
> +#define OV5647_ROW_START		0x01
> +#define OV5647_ROW_START_MIN		0
> +#define OV5647_ROW_START_MAX		2004
> +#define OV5647_ROW_START_DEF		54
> +
> +#define OV5647_COLUMN_START		0x02
> +#define OV5647_COLUMN_START_MIN		0
> +#define OV5647_COLUMN_START_MAX		2750
> +#define OV5647_COLUMN_START_DEF		16
> +
> +#define OV5647_WINDOW_HEIGHT		0x03
> +#define OV5647_WINDOW_HEIGHT_MIN	2
> +#define OV5647_WINDOW_HEIGHT_MAX	2006
> +#define OV5647_WINDOW_HEIGHT_DEF	1944
> +
> +#define OV5647_WINDOW_WIDTH		0x04
> +#define OV5647_WINDOW_WIDTH_MIN		2
> +#define OV5647_WINDOW_WIDTH_MAX		2752
> +#define OV5647_WINDOW_WIDTH_DEF		2592
> +
> +struct regval_list {
> +	u16 addr;
> +	u8 data;
> +};
> +
> +struct cfg_array {
> +	struct regval_list *regs;
> +	int size;
> +};
> +
> +struct sensor_win_size {
> +	int width;
> +	int height;
> +	unsigned int hoffset;
> +	unsigned int voffset;
> +	unsigned int hts;
> +	unsigned int vts;
> +	unsigned int pclk;
> +	unsigned int mipi_bps;
> +	unsigned int fps_fixed;
> +	unsigned int bin_factor;
> +	unsigned int intg_min;
> +	unsigned int intg_max;
> +	void *regs;
> +	int regs_size;
> +	int (*set_size)(struct v4l2_subdev *sd);
> +};
> +
> +
> +struct ov5647 {
> +	struct device			*dev;
> +	struct v4l2_subdev		sd;
> +	struct media_pad		pad;
> +	struct mutex			lock;
> +	struct v4l2_mbus_framefmt	format;
> +	struct sensor_format_struct	*fmt;
> +	unsigned int			width;
> +	unsigned int			height;
> +	unsigned int			capture_mode;
> +	int				hue;

At least capture_mode and hue are unused. Please remove unused fields.

> +	struct v4l2_fract		tpf;
> +	struct sensor_win_size		*current_wins;
> +};
> +
> +static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
> +{
> +	return container_of(sd, struct ov5647, sd);
> +}
> +
> +static struct regval_list sensor_oe_disable_regs[] = {
> +	{0x3000, 0x00},
> +	{0x3001, 0x00},
> +	{0x3002, 0x00},
> +};
> +
> +static struct regval_list sensor_oe_enable_regs[] = {
> +	{0x3000, 0x0f},
> +	{0x3001, 0xff},
> +	{0x3002, 0xe4},
> +};
> +
> +static struct regval_list ov5647_640x480[] = {

Does this list expect a certain external clock frequency? If it does, should
you check that the actual frequency matches with the expectation?

> +	{0x0100, 0x00},
> +	{0x0103, 0x01},
> +	{0x3034, 0x08},
> +	{0x3035, 0x21},
> +	{0x3036, 0x46},
> +	{0x303c, 0x11},
> +	{0x3106, 0xf5},
> +	{0x3821, 0x07},
> +	{0x3820, 0x41},
> +	{0x3827, 0xec},
> +	{0x370c, 0x0f},
> +	{0x3612, 0x59},
> +	{0x3618, 0x00},
> +	{0x5000, 0x06},
> +	{0x5001, 0x01},
> +	{0x5002, 0x41},
> +	{0x5003, 0x08},
> +	{0x5a00, 0x08},
> +	{0x3000, 0x00},
> +	{0x3001, 0x00},
> +	{0x3002, 0x00},
> +	{0x3016, 0x08},
> +	{0x3017, 0xe0},
> +	{0x3018, 0x44},
> +	{0x301c, 0xf8},
> +	{0x301d, 0xf0},
> +	{0x3a18, 0x00},
> +	{0x3a19, 0xf8},
> +	{0x3c01, 0x80},
> +	{0x3b07, 0x0c},
> +	{0x380c, 0x07},
> +	{0x380d, 0x68},
> +	{0x380e, 0x03},
> +	{0x380f, 0xd8},
> +	{0x3814, 0x31},
> +	{0x3815, 0x31},
> +	{0x3708, 0x64},
> +	{0x3709, 0x52},
> +	{0x3808, 0x02},
> +	{0x3809, 0x80},
> +	{0x380a, 0x01},
> +	{0x380b, 0xE0},
> +	{0x3801, 0x00},
> +	{0x3802, 0x00},
> +	{0x3803, 0x00},
> +	{0x3804, 0x0a},
> +	{0x3805, 0x3f},
> +	{0x3806, 0x07},
> +	{0x3807, 0xa1},
> +	{0x3811, 0x08},
> +	{0x3813, 0x02},
> +	{0x3630, 0x2e},
> +	{0x3632, 0xe2},
> +	{0x3633, 0x23},
> +	{0x3634, 0x44},
> +	{0x3636, 0x06},
> +	{0x3620, 0x64},
> +	{0x3621, 0xe0},
> +	{0x3600, 0x37},
> +	{0x3704, 0xa0},
> +	{0x3703, 0x5a},
> +	{0x3715, 0x78},
> +	{0x3717, 0x01},
> +	{0x3731, 0x02},
> +	{0x370b, 0x60},
> +	{0x3705, 0x1a},
> +	{0x3f05, 0x02},
> +	{0x3f06, 0x10},
> +	{0x3f01, 0x0a},
> +	{0x3a08, 0x01},
> +	{0x3a09, 0x27},
> +	{0x3a0a, 0x00},
> +	{0x3a0b, 0xf6},
> +	{0x3a0d, 0x04},
> +	{0x3a0e, 0x03},
> +	{0x3a0f, 0x58},
> +	{0x3a10, 0x50},
> +	{0x3a1b, 0x58},
> +	{0x3a1e, 0x50},
> +	{0x3a11, 0x60},
> +	{0x3a1f, 0x28},
> +	{0x4001, 0x02},
> +	{0x4004, 0x02},
> +	{0x4000, 0x09},
> +	{0x4837, 0x24},
> +	{0x4050, 0x6e},
> +	{0x4051, 0x8f},
> +	{0x0100, 0x01},
> +};
> +
> +struct sensor_format_struct;
> +
> +/**
> + * @short I2C Write operation
> + * @param[in] i2c_client I2C client
> + * @param[in] reg register address
> + * @param[in] val value to write
> + * @return Error code
> + */
> +static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
> +{
> +	int ret;
> +	unsigned char data[3] = { reg >> 8, reg & 0xff, val};
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	ret = i2c_master_send(client, data, 3);
> +	if (ret != 3) {
> +		dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
> +				__func__, reg);
> +		return ret < 0 ? ret : -EIO;
> +	}
> +	return 0;
> +}
> +
> +/**
> + * @short I2C Read operation
> + * @param[in] i2c_client I2C client
> + * @param[in] reg register address
> + * @param[out] val value read
> + * @return Error code
> + */
> +static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
> +{
> +	int ret;
> +	unsigned char data_w[2] = { reg >> 8, reg & 0xff };
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +
> +	ret = i2c_master_send(client, data_w, 2);
> +
> +	if (ret < 2) {
> +		dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
> +			__func__, reg);
> +		return ret < 0 ? ret : -EIO;
> +	}
> +
> +
> +	ret = i2c_master_recv(client, val, 1);
> +
> +	if (ret < 1) {
> +		dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
> +				__func__, reg);
> +		return ret < 0 ? ret : -EIO;
> +	}
> +	return 0;
> +}
> +
> +static int ov5647_write_array(struct v4l2_subdev *sd,
> +				struct regval_list *regs, int array_size)
> +{
> +	int i = 0;
> +	int ret = 0;
> +
> +	if (!regs)
> +		return -EINVAL;
> +
> +	while (i < array_size) {
> +		ret = ov5647_write(sd, regs->addr, regs->data);
> +		if (ret < 0)
> +			return ret;
> +		i++;
> +		regs++;
> +	}
> +	return 0;
> +}
> +
> +static void ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
> +{
> +	u8 channel_id;
> +
> +	ov5647_read(sd, 0x4814, &channel_id);
> +	channel_id &= ~(3 << 6);
> +	ov5647_write(sd, 0x4814, channel_id | (channel << 6));
> +}
> +
> +void ov5647_stream_on(struct v4l2_subdev *sd)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	ov5647_write(sd, 0x4202, 0x00);
> +	dev_dbg(&client->dev, "Stream on");
> +	ov5647_write(sd, 0x300D, 0x00);
> +}
> +
> +void ov5647_stream_off(struct v4l2_subdev *sd)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	ov5647_write(sd, 0x4202, 0x0f);
> +	dev_dbg(&client->dev, "Stream off");
> +	ov5647_write(sd, 0x300D, 0x01);
> +}
> +
> +/****************************************************************************/
> +
> +/**
> + * @short Set SW standby
> + * @param[in] sd v4l2 sd
> + * @param[in] stanby standby mode status (on or off)
> + * @return Error code
> + */
> +static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
> +{
> +	int ret;
> +	unsigned char rdval;
> +
> +	ret = ov5647_read(sd, 0x0100, &rdval);
> +	if (ret != 0)
> +		return ret;
> +
> +	if (standby)
> +		rdval &= 0xfe;
> +	else
> +		rdval |= 0x01;
> +
> +	ret = ov5647_write(sd, 0x0100, rdval);
> +
> +	return ret;
> +}
> +
> +/**
> + * @short Store information about the video data format.
> + */
> +static struct sensor_format_struct {
> +	u8 *desc;
> +	u32 mbus_code;
> +	struct regval_list *regs;
> +	int regs_size;
> +	int bpp;

At least desc and bpp are unused.

> +} sensor_formats[] = {
> +	{
> +		.desc		= "Raw RGB Bayer",
> +		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
> +		.regs		= ov5647_640x480,
> +		.regs_size	= ARRAY_SIZE(ov5647_640x480),
> +		.bpp		= 1
> +	},
> +};
> +#define N_FMTS ARRAY_SIZE(sensor_formats)
> +
> +/* ----------------------------------------------------------------------- */
> +
> +/**
> + * @short Initialize sensor
> + * @param[in] sd v4l2 subdev
> + * @param[in] val not used
> + * @return Error code
> + */
> +static int __sensor_init(struct v4l2_subdev *sd)
> +{
> +	int ret;
> +	u8 resetval;
> +	u8 rdval;
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	dev_dbg(&client->dev, "sensor init\n");
> +
> +	ret = ov5647_read(sd, 0x0100, &rdval);
> +	if (ret != 0)
> +		return ret;
> +
> +	ov5647_write(sd, 0x4800, 0x25);
> +	ov5647_stream_off(sd);
> +
> +	ret = ov5647_write_array(sd, ov5647_640x480,
> +					ARRAY_SIZE(ov5647_640x480));
> +	if (ret < 0) {
> +		dev_err(&client->dev, "write sensor_default_regs error\n");
> +		return ret;
> +	}
> +
> +	ov5647_set_virtual_channel(sd, 0);
> +
> +	ov5647_read(sd, 0x0100, &resetval);
> +	if (!(resetval & 0x01)) {
> +		dev_err(&client->dev, "Device was in SW standby");
> +		ov5647_write(sd, 0x0100, 0x01);
> +	}
> +
> +	ov5647_write(sd, 0x4800, 0x04);
> +	ov5647_stream_on(sd);
> +
> +	return 0;
> +}
> +
> +/**
> + * @short Control sensor power state
> + * @param[in] sd v4l2 subdev
> + * @param[in] on Sensor power
> + * @return Error code
> + */
> +static int sensor_power(struct v4l2_subdev *sd, int on)
> +{
> +	int ret;
> +	struct ov5647 *ov5647 = to_state(sd);
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	ret = 0;
> +	mutex_lock(&ov5647->lock);
> +
> +	if (on)	{
> +		dev_dbg(&client->dev, "OV5647 power on\n");
> +
> +		ret = ov5647_write_array(sd, sensor_oe_enable_regs,
> +				ARRAY_SIZE(sensor_oe_enable_regs));
> +
> +		ret = __sensor_init(sd);
> +
> +		if (ret < 0)
> +			dev_err(&client->dev,
> +				"Camera not available, check Power\n");
> +	} else {
> +		dev_dbg(&client->dev, "OV5647 power off\n");
> +
> +		dev_dbg(&client->dev, "disable oe\n");
> +		ret = ov5647_write_array(sd, sensor_oe_disable_regs,
> +				ARRAY_SIZE(sensor_oe_disable_regs));
> +
> +		if (ret < 0)
> +			dev_dbg(&client->dev, "disable oe failed\n");
> +
> +		ret = set_sw_standby(sd, true);
> +
> +		if (ret < 0)
> +			dev_dbg(&client->dev, "soft stby failed\n");
> +
> +	}
> +
> +	mutex_unlock(&ov5647->lock);
> +
> +	return ret;
> +}
> +
> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +/**
> + * @short Get register value
> + * @param[in] sd v4l2 subdev
> + * @param[in] reg register struct
> + * @return Error code
> + */
> +static int sensor_get_register(struct v4l2_subdev *sd,
> +				struct v4l2_dbg_register *reg)
> +{
> +	unsigned char val = 0;
> +	int ret;
> +
> +	ret = ov5647_read(sd, reg->reg & 0xff, &val);
> +	if (ret != 0)
> +		return ret;
> +
> +	reg->val = val;
> +	reg->size = 1;
> +
> +	return ret;
> +}
> +
> +/**
> + * @short Set register value
> + * @param[in] sd v4l2 subdev
> + * @param[in] reg register struct
> + * @return Error code
> + */
> +static int sensor_set_register(struct v4l2_subdev *sd,
> +				const struct v4l2_dbg_register *reg)
> +{
> +	return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
> +}
> +#endif
> +
> +/* ----------------------------------------------------------------------- */
> +
> +/**
> + * @short Subdev core operations registration
> + */
> +static const struct v4l2_subdev_core_ops sensor_core_ops = {
> +	.s_power		= sensor_power,

The s_power() op will be called by the bridge (ISP) driver as well. You
should expect that it may be enabled more than once before being disabled
equal number of times.

> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> +	.g_register		= sensor_get_register,
> +	.s_register		= sensor_set_register,
> +#endif
> +};
> +
> +/* ----------------------------------------------------------------------- */
> +
> +
> +
> +/**
> + * @short Enumerate available image formats
> + * @param[in] sd v4l2 subdev
> + * @param[in] index index
> + * @param[in] code MBUS Pixel code
> + * @return Error code
> + */
> +static int sensor_enum_fmt(struct v4l2_subdev *sd,
> +		struct v4l2_subdev_pad_config *cfg,
> +		struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	if (code->pad || code->index >= N_FMTS)
> +		return -EINVAL;
> +
> +	code->code = sensor_formats[code->index].mbus_code;
> +	return 0;
> +}
> +
> +/**
> + * @short Try frame format internal function
> + * @param[in] sd v4l2 subdev
> + * @param[in] fmt frame format
> + * @return Error code
> + */
> +static int sensor_try_fmt_internal(struct v4l2_subdev *sd,
> +	struct v4l2_mbus_framefmt *fmt, struct sensor_format_struct **ret_fmt,
> +	struct sensor_win_size **ret_wsize)
> +{
> +	int index;
> +
> +	for (index = 0; index < N_FMTS; index++)
> +		if (sensor_formats[index].mbus_code == fmt->code)
> +			break;
> +
> +	if (index >= N_FMTS)
> +		return -EINVAL;
> +
> +	if (ret_fmt != NULL)
> +		*ret_fmt = sensor_formats + index;
> +
> +	fmt->field = V4L2_FIELD_NONE;
> +
> +	return 0;
> +}
> +
> +/**
> + * @short Set frame format
> + * @param[in] sd v4l2 subdev
> + * @param[in] fmt frame format
> + * @return Error code
> + */
> +static int sensor_s_fmt(struct v4l2_subdev *sd,
> +		struct v4l2_subdev_pad_config *cfg,
> +		struct v4l2_subdev_format *fmt)
> +{
> +	int ret;
> +	struct sensor_format_struct *sensor_fmt;
> +	struct sensor_win_size *wsize;
> +	struct ov5647 *info = to_state(sd);
> +
> +	ov5647_write_array(sd, sensor_oe_disable_regs,
> +					ARRAY_SIZE(sensor_oe_disable_regs));

Should you check the error code here?

> +
> +	ret = sensor_try_fmt_internal(sd, &fmt->format,
> +					&sensor_fmt, &wsize);

Do you set wsize somewhere?

> +	if (ret)
> +		return ret;
> +
> +	ov5647_write_array(sd, sensor_fmt->regs, sensor_fmt->regs_size);

And here.

> +
> +	ret = 0;

ret was already zero.

> +
> +	if (wsize->regs)
> +		ov5647_write_array(sd, wsize->regs, wsize->regs_size);
> +
> +	if (wsize->set_size)
> +		wsize->set_size(sd);
> +
> +	info->fmt = sensor_fmt;
> +	info->width = wsize->width;
> +	info->height = wsize->height;
> +
> +	ov5647_write_array(sd, sensor_oe_enable_regs,
> +				ARRAY_SIZE(sensor_oe_enable_regs));
> +
> +	return 0;
> +}
> +
> +/**
> + * @short Set stream parameters
> + * @param[in] sd v4l2 subdev
> + * @param[in] parms stream parameters
> + * @return Error code
> + */
> +static int sensor_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +{
> +	struct v4l2_captureparm *cp = &parms->parm.capture;
> +	struct ov5647 *info = to_state(sd);
> +
> +	if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;
> +
> +	if (info->tpf.numerator == 0)
> +		return -EINVAL;
> +
> +	info->capture_mode = cp->capturemode;
> +
> +	return 0;
> +}
> +
> +/**
> + * @short Get stream parameters
> + * @param[in] sd v4l2 subdev
> + * @param[in] parms stream parameters
> + * @return Error code
> + */
> +static int sensor_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> +{
> +	struct v4l2_captureparm *cp = &parms->parm.capture;
> +	struct ov5647 *info = to_state(sd);
> +
> +	if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;
> +
> +	memset(cp, 0, sizeof(struct v4l2_captureparm));
> +	cp->capability = V4L2_CAP_TIMEPERFRAME;
> +	cp->capturemode = info->capture_mode;
> +
> +	return 0;
> +}
> +
> +/**
> + * @short Subdev video operations registration
> + *
> + */
> +static const struct v4l2_subdev_video_ops sensor_video_ops = {
> +	.s_parm		= sensor_s_parm,
> +	.g_parm		= sensor_g_parm,

Please use the g/s_frame_interval() instead. That's what sub-device drivers
generally use for the purpose.

> +};
> +
> +/* ----------------------------------------------------------------------- */
> +
> +/**
> + * @short Subdev operations registration
> + *
> + */
> +static const struct v4l2_subdev_ops subdev_ops = {
> +	.core			= &sensor_core_ops,
> +	.video			= &sensor_video_ops,
> +};
> +
> +/* -----------------------------------------------------------------------------
> + * V4L2 subdev internal operations
> + */
> +
> +/**
> + * @short Detect camera version and model
> + * @param[in] sd v4l2 subdev
> + * @return Error code
> + */
> +int ov5647_detect(struct v4l2_subdev *sd)
> +{
> +	unsigned char v;
> +	int ret;
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
> +	if (ret < 0)
> +		return ret;
> +	ret = ov5647_read(sd, OV5647_REG_CHIPID_H, &v);
> +	if (ret < 0)
> +		return ret;
> +	if (v != 0x56) {
> +		dev_err(&client->dev, "Wrong model version detected");
> +		return -ENODEV;
> +	}
> +	ret = ov5647_read(sd, OV5647_REG_CHIPID_L, &v);
> +	if (ret < 0)
> +		return ret;
> +	if (v != 0x47) {
> +		dev_err(&client->dev, "Wrong model version detected");
> +		return -ENODEV;
> +	}
> +
> +	ret = ov5647_write(sd, OV5647_SW_RESET, 0x00);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/**
> + * @short Detect if camera is registered
> + * @param[in] sd v4l2 subdev
> + * @return Error code
> + */
> +static int ov5647_registered(struct v4l2_subdev *sd)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(sd);
> +
> +	dev_info(&client->dev, "OV5647 detected at address 0x%02x\n",
> +				client->addr);

I might omit this. If there's a need to debug this then the information
should be printed by the framework instead using debug level messages.

> +
> +	return 0;
> +}
> +
> +/**
> + * @short Open device
> + * @param[in] sd v4l2 subdev
> + * @param[in] fh v4l2 file handler
> + * @return Error code
> + */
> +static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> +	struct v4l2_mbus_framefmt *format =
> +				v4l2_subdev_get_try_format(sd, fh->pad, 0);
> +	struct v4l2_rect *crop =
> +				v4l2_subdev_get_try_crop(sd, fh->pad, 0);
> +
> +	crop->left = OV5647_COLUMN_START_DEF;
> +	crop->top = OV5647_ROW_START_DEF;
> +	crop->width = OV5647_WINDOW_WIDTH_DEF;
> +	crop->height = OV5647_WINDOW_HEIGHT_DEF;
> +
> +	format->code = MEDIA_BUS_FMT_SBGGR8_1X8;
> +
> +	format->width = OV5647_WINDOW_WIDTH_DEF;
> +	format->height = OV5647_WINDOW_HEIGHT_DEF;
> +	format->field = V4L2_FIELD_NONE;
> +	format->colorspace = V4L2_COLORSPACE_SRGB;
> +
> +	return sensor_power(sd, true);
> +}
> +
> +/**
> + * @short Open device
> + * @param[in] sd v4l2 subdev
> + * @param[in] fh v4l2 file handler
> + * @return Error code
> + */
> +static int ov5647_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> +{
> +	return sensor_power(sd, false);
> +}
> +
> +/**
> + * @short Subdev internal operations registration
> + *
> + */
> +static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
> +	.registered = ov5647_registered,
> +	.open = ov5647_open,
> +	.close = ov5647_close,
> +};
> +
> +/**
> + * @short Initialization routine - Entry point of the driver
> + * @param[in] client pointer to the i2c client structure
> + * @param[in] id pointer to the i2c device id structure
> + * @return 0 on success and a negative number on failure
> + */
> +static int ov5647_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	struct ov5647 *sensor;
> +	int ret = 0;

No need to initialise ret here.

> +	struct v4l2_subdev *sd;
> +
> +	dev_info(&client->dev, "Installing OmniVision OV5647 camera driver\n");
> +
> +	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
> +	if (sensor == NULL)
> +		return -ENOMEM;
> +
> +	mutex_init(&sensor->lock);
> +	sensor->dev = dev;
> +
> +	sd = &sensor->sd;
> +	v4l2_i2c_subdev_init(sd, client, &subdev_ops);
> +	sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
> +	sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
> +	ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
> +	if (ret < 0)
> +		goto mutex_remove;
> +
> +	ret = ov5647_detect(sd);
> +	if (ret < 0)
> +		goto error;
> +
> +	ret = v4l2_async_register_subdev(sd);
> +	if (ret < 0)
> +		goto error;
> +
> +	return 0;
> +error:
> +	media_entity_cleanup(&sd->entity);
> +mutex_remove:
> +	mutex_destroy(&sensor->lock);
> +	return ret;
> +}
> +
> +/**
> + * @short Exit routine - Exit point of the driver
> + * @param[in] client pointer to the i2c client structure
> + * @return 0 on success and a negative number on failure
> + */
> +static int ov5647_remove(struct i2c_client *client)
> +{
> +	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> +	struct ov5647 *ov5647 = to_state(sd);
> +
> +	v4l2_async_unregister_subdev(&ov5647->sd);
> +	media_entity_cleanup(&ov5647->sd.entity);
> +	v4l2_device_unregister_subdev(sd);
> +	mutex_destroy(&ov5647->lock);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ov5647_id[] = {
> +	{ "ov5647", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ov5647_id);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id ov5647_of_match[] = {
> +	{ .compatible = "ovti,ov5647" },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ov5647_of_match);
> +#endif
> +
> +/**
> + * @short i2c driver structure
> + */
> +static struct i2c_driver ov5647_driver = {
> +	.driver = {
> +		.of_match_table = of_match_ptr(ov5647_of_match),
> +		.owner	= THIS_MODULE,
> +		.name	= "ov5647",
> +	},
> +	.probe		= ov5647_probe,
> +	.remove		= ov5647_remove,
> +	.id_table	= ov5647_id,
> +};
> +
> +module_i2c_driver(ov5647_driver);
> +
> +MODULE_AUTHOR("Ramiro Oliveira <roliveir@synopsys.com>");
> +MODULE_DESCRIPTION("A low-level driver for OmniVision ov5647 sensors");
> +MODULE_LICENSE("GPL v2");

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply

* Re: [PATCH 05/16] drivers/fsi: Add fake master driver
From: Jeremy Kerr @ 2016-12-07 23:27 UTC (permalink / raw)
  To: Mark Rutland, Chris Bostic
  Cc: devicetree, linux-kernel, geert+renesas, andrew, gregkh,
	mturquette, linux, Chris Bostic, sre, robh+dt, benh, joel,
	alistair, linux-arm-kernel
In-Reply-To: <20161207120905.GD7054@leverpostej>

Hi Mark & Chris,

> On Tue, Dec 06, 2016 at 06:14:26PM -0600, Chris Bostic wrote:
>> From: Jeremy Kerr <jk@ozlabs.org>
>>
>> For debugging, add a fake master driver, that only supports reads,
>> returning a fixed set of data.
> 
>> +config FSI_MASTER_FAKE
>> +	tristate "Fake FSI master"
>> +	depends on FSI
>> +	---help---
>> +	This option enables a fake FSI master driver for debugging.
>> +endif
> 
>> +static const struct of_device_id fsi_master_fake_match[] = {
>> +	{ .compatible = "ibm,fsi-master-fake" },
>> +	{ },
>> +};
> 
> NAK.
> 
> DT should be treated as an ABI, and should describe the HW explicitly.
> This makes no sense. This is also missing a binding document.
> 
> Have your module take a module parameter allowing you to bind it to
> arbitrary devices, or do something like what PCI does where you can
> bind/unbind arbitrary drivers to devices using sysfs.

This driver is purely for testing the FSI engine scan code; we could
probably just drop this patch since I suspect that it's no longer useful
(now that we have an actual master driver).

If we do want to keep it though, I'd say we remove the device tree
dependency; all this is doing at the moment is triggering the ->probe,
and there are better ways to do that.

Cheers,


Jeremy

^ permalink raw reply

* Re: [PATCH 08/16] drivers/fsi: Add crc4 helpers
From: Jeremy Kerr @ 2016-12-07 23:33 UTC (permalink / raw)
  To: Greg KH, Chris Bostic
  Cc: robh+dt, mark.rutland, linux, sre, mturquette, geert+renesas,
	devicetree, linux-arm-kernel, joel, linux-kernel, andrew,
	alistair, benh, Chris Bostic
In-Reply-To: <20161207090219.GA14742@kroah.com>

Hi Greg,

> Why not just create lib/crc4.c with these functions, like the other crc
> functions in the kernel?

Two (bad) reasons:

 - The crc4 implementation here is pretty specific to the FSI
   usage (only supporting 4-bit-sized chunks), to keep the math & lookup
   table simple

 - I'm lazy

So yes, we should spend the effort now to make this generic enough for
a lib/crc4.c. Would we want to support different values for the
polynomial?

Chris: do you want me to to that, or will you?

Cheers,


Jeremy

^ permalink raw reply

* Re: [PATCH] PM / Domains: Fix compatible for domain idle state
From: Rafael J. Wysocki @ 2016-12-08  0:13 UTC (permalink / raw)
  To: Ulf Hansson, Rob Herring, Lina Iyer
  Cc: Kevin Hilman, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Andy Gross, Stephen Boyd,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Brendan Jackman, Lorenzo Pieralisi, Sudeep Holla, Juri Lelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1616414.c1s7NxTqJS-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>

On Thursday, December 01, 2016 10:21:59 PM Rafael J. Wysocki wrote:
> On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
> > On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> > >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > >> > Re-using idle state definition provided by arm,idle-state for domain
> > >> > idle states creates a lot of confusion and limits further evolution of
> > >> > the domain idle definition. To keep things clear and simple, define a
> > >> > idle states for domain using a new compatible "domain-idle-state".
> > >> >
> > >> > Fix existing PM domains code to look for the newly defined compatible.
> > >> >
> > >> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > >> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > >> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > >> > ---
> > >> >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
> > >> >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
> > >> >  drivers/base/power/domain.c                        |  2 +-
> > >> >  3 files changed, 38 insertions(+), 5 deletions(-)
> > >> >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > new file mode 100644
> > >> > index 0000000..eefc7ed
> > >> > --- /dev/null
> > >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > @@ -0,0 +1,33 @@
> > >> > +PM Domain Idle State Node:
> > >> > +
> > >> > +A domain idle state node represents the state parameters that will be used to
> > >> > +select the state when there are no active components in the domain.
> > >> > +
> > >> > +The state node has the following parameters -
> > >> > +
> > >> > +- compatible:
> > >> > +       Usage: Required
> > >> > +       Value type: <string>
> > >> > +       Definition: Must be "domain-idle-state".
> > >> > +
> > >> > +- entry-latency-us
> > >> > +       Usage: Required
> > >> > +       Value type: <prop-encoded-array>
> > >> > +       Definition: u32 value representing worst case latency in
> > >> > +                   microseconds required to enter the idle state.
> > >> > +                   The exit-latency-us duration may be guaranteed
> > >> > +                   only after entry-latency-us has passed.
> > >>
> > >> As we anyway are going to change this, why not use an u64 and have the
> > >> value in ns instead of us?
> > >
> > > I can't imagine that you would need more resolution or range. For times
> > > less than 1us, s/w and register access times are going to dominate the
> > > time.
> > >
> > > Unless there is a real need, I'd keep alignment with the existing
> > > binding.
> > 
> > Rob, are you fine with this? I thought it would be great to get this
> > in for 4.10 rc1.
> 
> Rob, any objections here?

Well, no objections, so applied.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ 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