Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Sekhar Nori @ 2016-11-11 10:47 UTC (permalink / raw)
  To: Rob Herring, Bartosz Golaszewski
  Cc: Mark Rutland, linux-devicetree, David Airlie, Kevin Hilman,
	Michael Turquette, Russell King, linux-drm, LKML, Tomi Valkeinen,
	Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <20161109182430.yh6uqeu2ufzkctww@rob-hp-laptop>

On Wednesday 09 November 2016 11:54 PM, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 03:45:34PM +0100, Bartosz Golaszewski wrote:
>> Create a new driver for the da8xx DDR2/mDDR controller and implement
>> support for writing to the Peripheral Bus Burst Priority Register.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  .../memory-controllers/ti-da8xx-ddrctl.txt         |  20 +++
> 
> Acked-by: Rob Herring <robh@kernel.org>

Applied to v4.10/drivers

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH v2 2/5] ARM: bus: da8xx-mstpri: new driver
From: Sekhar Nori @ 2016-11-11 10:47 UTC (permalink / raw)
  To: Rob Herring, Bartosz Golaszewski
  Cc: Mark Rutland, linux-devicetree, David Airlie, Kevin Hilman,
	Michael Turquette, Russell King, linux-drm, LKML, Tomi Valkeinen,
	Jyri Sarha, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <20161109182433.vskk5rn7hfxshfy6@rob-hp-laptop>

On Wednesday 09 November 2016 11:54 PM, Rob Herring wrote:
> On Mon, Oct 31, 2016 at 03:45:35PM +0100, Bartosz Golaszewski wrote:
>> Create the driver for the da8xx master peripheral priority
>> configuration and implement support for writing to the three
>> Master Priority registers on da850 SoCs.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
> 
> Acked-by: Rob Herring <robh@kernel.org>

Applied to v4.10/drivers

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: liviu.dudau @ 2016-11-11 10:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Gabriele Paoloni, linux-arm-kernel@lists.infradead.org,
	Yuanzhichang, mark.rutland@arm.com, devicetree@vger.kernel.org,
	lorenzo.pieralisi@arm.com, minyard@acm.org,
	linux-pci@vger.kernel.org, benh@kernel.crashing.org, John Garry,
	will.deacon@arm.com, linux-kernel@vger.kernel.org, xuwei (O),
	Linuxarm, zourongrong@gmail.com, robh+dt@kernel.org
In-Reply-To: <10334260.ztLXZ2Oynd@wuerfel>

Hi Arnd,

On Thu, Nov 10, 2016 at 05:07:21PM +0100, Arnd Bergmann wrote:
> On Thursday, November 10, 2016 3:36:49 PM CET Gabriele Paoloni wrote:
> > 
> > Where should we get the range from? For LPC we know that it is going
> > Work on anything that is not used by PCI I/O space, and this is 
> > why we use [0, PCIBIOS_MIN_IO]
> 
> It should be allocated the same way we allocate PCI config space
> segments. This is currently done with the io_range list in
> drivers/pci/pci.c, which isn't perfect but could be extended
> if necessary. Based on what others commented here, I'd rather
> make the differences between ISA/LPC and PCI I/O ranges smaller
> than larger.
> 
> > > Your current version has
> > > 
> > >         if (arm64_extio_ops->pfout)                             \
> > >                 arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > >                        addr, value, sizeof(type));             \
> > > 
> > > Instead, just subtract the start of the range from the logical
> > > port number to transform it back into a bus-local port number:
> > 
> > These accessors do not operate on IO tokens:
> > 
> > If (arm64_extio_ops->start > addr || arm64_extio_ops->end < addr)
> > addr is not going to be an I/O token; in fact patch 2/3 imposes that
> > the I/O tokens will start at PCIBIOS_MIN_IO. So from 0 to PCIBIOS_MIN_IO
> > we have free physical addresses that the accessors can operate on.
> 
> Ah, I missed that part. I'd rather not use PCIBIOS_MIN_IO to refer to
> the logical I/O tokens, the purpose of that macro is really meant
> for allocating PCI I/O port numbers within the address space of
> one bus.
> 
> Note that it's equally likely that whichever next platform needs
> non-mapped I/O access like this actually needs them for PCI I/O space,
> and that will use it on addresses registered to a PCI host bridge.
> 
> If we separate the two steps:
> 
> a) assign a range of logical I/O port numbers to a bus

Except that currently when we add ranges to io_range_list we don't have
a bus number yet, because the parsing happens before the host bridge
has been created. Maybe register_io_range() can take a bus number as an
argument, but I'm not sure how we are going to use that in pci_pio_to_address()
or pci_address_to_pio().

Best regards,
Liviu

> b) register a set of helpers for redirecting logical I/O
>    port to a helper function
> 
> then I think the code will get cleaner and more flexible.
> It should actually then be able to replace the powerpc
> specific implementation.
> 
> 	Arnd

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply

* Re: [PATCH] [media] mtk-mdp: allocate video_device dynamically
From: Hans Verkuil @ 2016-11-11 10:51 UTC (permalink / raw)
  To: Minghsiu Tsai, Hans Verkuil, daniel.thompson, Rob Herring,
	Mauro Carvalho Chehab, Matthias Brugger, Daniel Kurtz,
	Pawel Osciak
  Cc: srv_heupstream, Eddie Huang, Yingjoe Chen, devicetree,
	linux-kernel, linux-arm-kernel, linux-media, linux-mediatek
In-Reply-To: <1478522529-57129-2-git-send-email-minghsiu.tsai@mediatek.com>

Almost correct:

On 11/07/2016 01:42 PM, Minghsiu Tsai wrote:
> It can fix known problems with embedded video_device structs.
> 
> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> ---
>  drivers/media/platform/mtk-mdp/mtk_mdp_core.h |  2 +-
>  drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c  | 33 ++++++++++++++++-----------
>  2 files changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> index 848569d..ad1cff3 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.h
> @@ -167,7 +167,7 @@ struct mtk_mdp_dev {
>  	struct mtk_mdp_comp		*comp[MTK_MDP_COMP_ID_MAX];
>  	struct v4l2_m2m_dev		*m2m_dev;
>  	struct list_head		ctx_list;
> -	struct video_device		vdev;
> +	struct video_device		*vdev;
>  	struct v4l2_device		v4l2_dev;
>  	struct workqueue_struct		*job_wq;
>  	struct platform_device		*vpu_dev;
> diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> index 9a747e7..b8dee1c 100644
> --- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> +++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
> @@ -1236,16 +1236,22 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
>  	int ret;
>  
>  	mdp->variant = &mtk_mdp_default_variant;
> -	mdp->vdev.device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> -	mdp->vdev.fops = &mtk_mdp_m2m_fops;
> -	mdp->vdev.ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> -	mdp->vdev.release = video_device_release_empty;
> -	mdp->vdev.lock = &mdp->lock;
> -	mdp->vdev.vfl_dir = VFL_DIR_M2M;
> -	mdp->vdev.v4l2_dev = &mdp->v4l2_dev;
> -	snprintf(mdp->vdev.name, sizeof(mdp->vdev.name), "%s:m2m",
> +	mdp->vdev = video_device_alloc();
> +	if (!mdp->vdev) {
> +		dev_err(dev, "failed to allocate video device\n");
> +		ret = -ENOMEM;
> +		goto err_video_alloc;
> +	}
> +	mdp->vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> +	mdp->vdev->fops = &mtk_mdp_m2m_fops;
> +	mdp->vdev->ioctl_ops = &mtk_mdp_m2m_ioctl_ops;
> +	mdp->vdev->release = video_device_release;
> +	mdp->vdev->lock = &mdp->lock;
> +	mdp->vdev->vfl_dir = VFL_DIR_M2M;
> +	mdp->vdev->v4l2_dev = &mdp->v4l2_dev;
> +	snprintf(mdp->vdev->name, sizeof(mdp->vdev->name), "%s:m2m",
>  		 MTK_MDP_MODULE_NAME);
> -	video_set_drvdata(&mdp->vdev, mdp);
> +	video_set_drvdata(mdp->vdev, mdp);
>  
>  	mdp->m2m_dev = v4l2_m2m_init(&mtk_mdp_m2m_ops);
>  	if (IS_ERR(mdp->m2m_dev)) {
> @@ -1254,26 +1260,27 @@ int mtk_mdp_register_m2m_device(struct mtk_mdp_dev *mdp)
>  		goto err_m2m_init;
>  	}
>  
> -	ret = video_register_device(&mdp->vdev, VFL_TYPE_GRABBER, 2);
> +	ret = video_register_device(mdp->vdev, VFL_TYPE_GRABBER, 2);
>  	if (ret) {
>  		dev_err(dev, "failed to register video device\n");
>  		goto err_vdev_register;
>  	}
>  
>  	v4l2_info(&mdp->v4l2_dev, "driver registered as /dev/video%d",
> -		  mdp->vdev.num);
> +		  mdp->vdev->num);
>  	return 0;
>  
>  err_vdev_register:
>  	v4l2_m2m_release(mdp->m2m_dev);
>  err_m2m_init:
> -	video_device_release(&mdp->vdev);
> +	video_unregister_device(mdp->vdev);

This should remain video_device_release: the video_register_device call failed, so
the device hasn't been registered. In that case just release the device (i.e.
free the allocated memory).

> +err_video_alloc:
>  
>  	return ret;
>  }
>  
>  void mtk_mdp_unregister_m2m_device(struct mtk_mdp_dev *mdp)
>  {
> -	video_device_release(&mdp->vdev);
> +	video_unregister_device(mdp->vdev);
>  	v4l2_m2m_release(mdp->m2m_dev);
>  }
> 

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH v5 03/23] usb: ulpi: Support device discovery via DT
From: Heikki Krogerus @ 2016-11-11 11:02 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-usb, linux-arm-kernel, linux-kernel, linux-arm-msm,
	Andy Gross, Bjorn Andersson, Neil Armstrong, Arnd Bergmann,
	Felipe Balbi, Peter Chen, Greg Kroah-Hartman, devicetree,
	Rob Herring
In-Reply-To: <20161018015636.11701-4-stephen.boyd@linaro.org>

On Mon, Oct 17, 2016 at 06:56:16PM -0700, Stephen Boyd wrote:
> The qcom HSIC ULPI phy doesn't have any bits set in the vendor or
> product ID registers. This makes it impossible to make a ULPI
> driver match against the ID registers. Add support to discover
> the ULPI phys via DT help alleviate this problem. In the DT case,
> we'll look for a ULPI bus node underneath the device registering
> the ULPI viewport (or the parent of that device to support
> chipidea's device layout) and then match up the phy node
> underneath that with the ULPI device that's created.
> 
> The side benefit of this is that we can use standard properties
> in the phy node like clks, regulators, gpios, etc. because we
> don't have firmware like ACPI to turn these things on for us. And
> we can use the DT phy binding to point our phy consumer to the
> phy provider.
> 
> The ULPI bus code supports native enumeration by reading the
> vendor ID and product ID registers at device creation time, but
> we can't be certain that those register reads will succeed if the
> phy is not powered up. To avoid any problems with reading the ID
> registers before the phy is powered we fallback to DT matching
> when the ID reads fail.
> 
> If the ULPI spec had some generic power sequencing for these
> registers we could put that into the ULPI bus layer and power up
> the device before reading the ID registers. Unfortunately this
> doesn't exist and the power sequence is usually device specific.
> By having the device matched up with DT we can avoid this
> problem.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

-- 
heikki

^ permalink raw reply

* Re: [RESEND PATCH v1 02/11] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Sysctrl and Djtag dts bindings
From: Anurup M @ 2016-11-11 11:19 UTC (permalink / raw)
  To: Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
	corbet-T1hC0tSOHrs, catalin.marinas-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, arnd-r2nGTMty4D4,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ, krzk-DgEjT+Ai2ygdnm+yROfE0A,
	anurup.m-hv44wF8Li93QT0dZR+AlfA,
	zhangshaokun-C8/M+/jPZTeaMJb+Lgu22Q,
	tanxiaojun-hv44wF8Li93QT0dZR+AlfA, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
	sanil.kumar-C8/M+/jPZTeaMJb+Lgu22Q,
	john.garry-hv44wF8Li93QT0dZR+AlfA,
	gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	shiju.jose-hv44wF8Li93QT0dZR+AlfA,
	wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA,
	guohanjun-hv44wF8Li93QT0dZR+AlfA, shyju.pv-hv44wF8Li93QT0dZR+AlfA,
	linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <20161110172320.GA10137@leverpostej>



On Thursday 10 November 2016 10:53 PM, Mark Rutland wrote:
> Hi,
>
> On Thu, Nov 03, 2016 at 01:41:58AM -0400, Anurup M wrote:
>> From: Tan Xiaojun <tanxiaojun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>
>> 	1) Add Hisilicon HiP05/06/07 CPU and ALGSUB system controller dts
>> 	   bindings.
>> 	2) Add Hisilicon Djtag dts binding.
>>
>> Signed-off-by: Tan Xiaojun <tanxiaojun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Anurup M <anurup.m-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>   .../bindings/arm/hisilicon/hisilicon.txt           | 82 ++++++++++++++++++++++
>>   1 file changed, 82 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>> index 7df79a7..341cbb9 100644
>> --- a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>> +++ b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>> @@ -270,3 +270,85 @@ Required Properties:
>>     [1]: bootwrapper size
>>     [2]: relocation physical address
>>     [3]: relocation size
>
>> +-----------------------------------------------------------------------
>> +The Hisilicon Djtag in CPU die is an independent component which connects with
>> +some other components in the SoC by Debug Bus. This driver can be configured
>> +to access the registers of connecting components (like L3 cache, l3 cache PMU
>> + etc.) during real time debugging by sysctrl. These components appear as child
>> +nodes of djtag.
> Please put the djtag binding in a new file.
>
> It's clearly unrelated to many other things in this file, which should
> also be split out.
Ok. Shall move the djtag bindings to hisilicon/djtag.txt.

>> +The Hip05/06/07 CPU system controller(sysctrl) support to manage some important
>> +components (such as clock, reset, soft reset, secure debugger, etc.).
>> +The CPU sysctrl registers in hip05/06/07 doesnot use syscon but will be mapped
>> +by djtag driver for use by connecting components.
> The djtag driver is irrelvant here.
>
> If there's a realtionship between the two, please define that in the
> binding rather than implicitly assuming it in the driver.
Ok. I shall remove "djtag driver" and shall modify as

+The Hisilicon Djtag in CPU die is an independent component which connects with
+some other components in the SoC by Debug Bus. The djtag will use the system controller
+registers to access the connecting components in CPU die (like L3 cache, l3 cache PMU
+etc.) during real time debugging. These components appear as child nodes of djtag.

>> +
>> +Required properties:
>> +  - compatible : "hisilicon,hip05-cpu-djtag-v1"
>> +  - reg : Register address and size
>> +
>> +Hisilicon HiP06 djtag for CPU sysctrl
>> +Required properties:
>> +- compatible : "hisilicon,hip06-sysctrl", "syscon", "simple-mfd";
> This looks messy. Why is this syscon and a simple-mfd?
>
> We should kill off / deprecate the syscon binding. It's completely
> meaningless.
My Apologies. The syscon is not used now. Mistake in file version used 
for patch creation.
The compatible filed will be "hisilicon,hisi-cpu-djtag-v1"
>> +- reg : Register address and size
>> +- djtag :
>> +  - compatible : "hisilicon,hip06-cpu-djtag-v1"
>> +  - reg : Register address and size
>> +
>> +Hisilicon HiP07 djtag for CPU sysctrl
>> +Required properties:
>> +  - compatible : "hisilicon,hip07-cpu-djtag-v2"
>> +  - reg : Register address and size
>> +
>> +Example:
>> +	/* for Hisilicon HiP05 djtag for CPU sysctrl */
>> +	djtag0: djtag@80010000 {
>> +		compatible = "hisilicon,hip05-cpu-djtag-v1";
>> +		reg = <0x0 0x80010000 0x0 0x10000>;
>> +
>> +		/* For L3 cache PMU */
>> +		pmul3c0 {
>> +			compatible = "hisilicon,hisi-pmu-l3c-v1";
>> +			scl-id = <0x02>;
>> +			num-events = <0x16>;
>> +			num-counters = <0x08>;
>> +			module-id = <0x04>;
>> +			num-banks = <0x04>;
>> +			cfgen-map = <0x02 0x04 0x01 0x08>;
>> +			counter-reg = <0x170>;
>> +			evctrl-reg = <0x04>;
>> +			event-en = <0x1000000>;
>> +			evtype-reg = <0x140>;
>> +		};
> This sub-node needs a binding document.
>
> These properties are completely opaque
The L3 cache PMU bindings are defined @bindings/arm/hisilicon/pmu.txt.
Is it OK that I document here(hisilicon/djtag.txt), a reference to the 
PMU bindings doc ?

>> +	};
>> +
>> +-----------------------------------------------------------------------
>> +The Hisilicon HiP05/06/07 ALGSUB system controller(sysctrl) is in IO die
>> +of SoC. It has a similar function as the Hisilicon HiP05/06/07 CPU system
>> +controller in CPU die and it manage different components, like RSA, etc.
>> +The Hisilicon Djtag in IO die has a similar function as in CPU die and maps
>> +the sysctrl registers for use by connecting components.
>> +All connecting components shall appear as child nodes of djtag.
> I don't follow the above. This describes an ALGSUB system controllerm
> but the documentation below is all about djtag.
The ALGSUB is the name of the sysctrl of IO die. I shall remove ALGSUB 
to avoid confusion.

I would also add the below details in the introduction.

The Hisilicon djtag will use the system controller registers to control 
access to connecting modules
of CPU and IO die in the chip. There are separate system controller 
registers for CPU and IO die  in the chip.

And the section will be modified as

+Hisilicon HiP05 djtag for IO sysctrl
+Required properties:
+  - compatible : "hisilicon,hisi-io-djtag-v1"
+  - reg : Register address and size
+
+Hisilicon HiP06/07 djtag for IO sysctrl
+Required properties:
+  - compatible : "hisilicon,hip06-io-djtag-v2"
+  - reg : Register address and size
+
+Example:
+	/* for Hisilicon HiP05 djtag for IO sysctrl */
+	djtag0: djtag@d0000000 {
+	       compatible = "hisilicon,hisi-io-djtag-v1";
+		reg = <0x0 0xd0000000 0x0 0x10000>;
+	};

Thanks,
Anurup

> Thanks,
> Mark.
>
>> +Hisilicon HiP05 djtag for ALGSUB sysctrl
>> +Required properties:
>> +  - compatible : "hisilicon,hip05-io-djtag-v1"
>> +  - reg : Register address and size
>> +
>> +Hisilicon HiP06 djtag for ALGSUB sysctrl
>> +Required properties:
>> +  - compatible : "hisilicon,hip06-io-djtag-v2"
>> +  - reg : Register address and size
>> +
>> +Hisilicon HiP07 djtag for ALGSUB sysctrl
>> +Required properties:
>> +  - compatible : "hisilicon,hip07-io-djtag-v2"
>> +  - reg : Register address and size
>> +
>> +Example:
>> +	/* for Hisilicon HiP05 djtag for alg sysctrl */
>> +	djtag0: djtag@d0000000 {
>> +	       compatible = "hisilicon,hip05-io-djtag-v1";
>> +		reg = <0x0 0xd0000000 0x0 0x10000>;
>> +	};
>> -- 
>> 2.1.4
>>

--
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

* [PATCH v3 0/3] ASoC driver for the TSE-850
From: Peter Rosin @ 2016-11-11 11:29 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	devicetree

Hi!

v2 -> v3 changes:
- update the preparatory atmel_ssc_dai patch with error checking and
  fallback handling of the CMR_DIV divider in addition to the new
  fallback handling of TCMR_PERIOD/RCMR_PERIOD from v2.

v1 -> v2 changes:
- new subject for the bindings patch to make it easier to find
- move the driver to the sound/soc/atmel directory
- add cached values for add/loop1/loop2 to avoid gets from output gpios
- use _cansleep when updating gpios
- add comment on how the regulator voltage is mapped to the ana enum
- drop the .hw_params hook that did set the cpu dai divider and...
- ...add a preparatory patch that does this in the cpu dai driver instead
- drop .init and set the dapm routes directly in the card struct instead


The TSE-850 is an FM Transmitter Station Equipment, designed to generate
baseband signals for FM, mainly the DARC subcarrier, but other signals
are also possible.

This adds a driver for the "sound" bits of the device (quoted since it
is normally not used for normal sound output, but that works too of
course). 

I have not provided a patch to add axentia as a devicetree vendor prefix,
since such a patch is already pending in an IIO series [1] that seems
close to being accepted.

However, there are a couple of points that I'm not 100% satisfied with
for this driver.

First, I do not know how to describe the relays that control if the
IN1/IN2 signals are directly routed towards OUT1/OUT2 or if they are
routed to the "add" switch. The dapm routing treats this as if the
IN1/IN2 signals are always routed to both the "add" switch and to
the muxes feeding OUT1/OUT2. This is fine with me since nothing is
powered in those sections anyway, so what dapm thinks does not really
matter. But it is a wart all the same.

Second, there's my comment in tse850_put_mix() when the "add" switch
is updated. I believe this update should really happen as a side
effect of the call to snd_soc_dapm_mixer_update_power(), so that it
happens at the right point compared to other stuff that is powered.
But I do not know how to hook that up and instead I flip the switch
before the call since it doesn't really matter. I.e., any noise
resulting from this badness is negligeble in practice.

Cheers,
Peter

[1] http://lwn.net/Articles/705931

Peter Rosin (3):
  ASoC: atmel_ssc_dai: if not provided, default to sensible dividers
  ASoC: tse850: document axentia,tse850-pcm5142 bindings
  ASoC: atmel: tse850: add ASoC driver for the Axentia TSE-850

 .../bindings/sound/axentia,tse850-pcm5142.txt      |  88 ++++
 MAINTAINERS                                        |   7 +
 sound/soc/atmel/Kconfig                            |  10 +
 sound/soc/atmel/Makefile                           |   2 +
 sound/soc/atmel/atmel_ssc_dai.c                    |  83 +++-
 sound/soc/atmel/atmel_ssc_dai.h                    |   1 +
 sound/soc/atmel/tse850-pcm5142.c                   | 472 +++++++++++++++++++++
 7 files changed, 656 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
 create mode 100644 sound/soc/atmel/tse850-pcm5142.c

-- 
2.1.4

^ permalink raw reply

* [PATCH v3 1/3] ASoC: atmel_ssc_dai: if not provided, default to sensible dividers
From: Peter Rosin @ 2016-11-11 11:29 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478863752-10569-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

When this driver masters BCLK and/or LRCLK, and noone has stated
differently, assume that all the bits of a frame are used.

This relieves the cpu dai users from the duty to fill in the dividers for
the common case.

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 sound/soc/atmel/atmel_ssc_dai.c | 83 +++++++++++++++++++++++++++++++++++++----
 sound/soc/atmel/atmel_ssc_dai.h |  1 +
 2 files changed, 77 insertions(+), 7 deletions(-)

diff --git a/sound/soc/atmel/atmel_ssc_dai.c b/sound/soc/atmel/atmel_ssc_dai.c
index 16e459aedffe..a1e2c5682dcd 100644
--- a/sound/soc/atmel/atmel_ssc_dai.c
+++ b/sound/soc/atmel/atmel_ssc_dai.c
@@ -380,6 +380,7 @@ static void atmel_ssc_shutdown(struct snd_pcm_substream *substream,
 		ssc_writel(ssc_p->ssc->regs, CR, SSC_BIT(CR_SWRST));
 		/* Clear the SSC dividers */
 		ssc_p->cmr_div = ssc_p->tcmr_period = ssc_p->rcmr_period = 0;
+		ssc_p->forced_divider = 0;
 	}
 	spin_unlock_irq(&ssc_p->lock);
 
@@ -426,14 +427,17 @@ static int atmel_ssc_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
 		else
 			if (div != ssc_p->cmr_div)
 				return -EBUSY;
+		ssc_p->forced_divider |= BIT(ATMEL_SSC_CMR_DIV);
 		break;
 
 	case ATMEL_SSC_TCMR_PERIOD:
 		ssc_p->tcmr_period = div;
+		ssc_p->forced_divider |= BIT(ATMEL_SSC_TCMR_PERIOD);
 		break;
 
 	case ATMEL_SSC_RCMR_PERIOD:
 		ssc_p->rcmr_period = div;
+		ssc_p->forced_divider |= BIT(ATMEL_SSC_RCMR_PERIOD);
 		break;
 
 	default:
@@ -443,6 +447,28 @@ static int atmel_ssc_set_dai_clkdiv(struct snd_soc_dai *cpu_dai,
 	return 0;
 }
 
+/* Is the cpu-dai master of the frame clock? */
+static int atmel_ssc_cfs(struct atmel_ssc_info *ssc_p)
+{
+	switch (ssc_p->daifmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBM_CFS:
+	case SND_SOC_DAIFMT_CBS_CFS:
+		return 1;
+	}
+	return 0;
+}
+
+/* Is the cpu-dai master of the bit clock? */
+static int atmel_ssc_cbs(struct atmel_ssc_info *ssc_p)
+{
+	switch (ssc_p->daifmt & SND_SOC_DAIFMT_MASTER_MASK) {
+	case SND_SOC_DAIFMT_CBS_CFM:
+	case SND_SOC_DAIFMT_CBS_CFS:
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * Configure the SSC.
  */
@@ -459,6 +485,9 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 	u32 tfmr, rfmr, tcmr, rcmr;
 	int ret;
 	int fslen, fslen_ext;
+	u32 cmr_div;
+	u32 tcmr_period;
+	u32 rcmr_period;
 
 	/*
 	 * Currently, there is only one set of dma params for
@@ -470,6 +499,46 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 	else
 		dir = 1;
 
+	/*
+	 * If the cpu dai should provide BCLK, but noone has provided the
+	 * divider needed for that to work, fall back to something sensible.
+	 */
+	cmr_div = ssc_p->cmr_div;
+	if (!(ssc_p->forced_divider & BIT(ATMEL_SSC_CMR_DIV)) &&
+	    atmel_ssc_cbs(ssc_p)) {
+		int bclk_rate = snd_soc_params_to_bclk(params);
+
+		if (bclk_rate < 0) {
+			dev_err(dai->dev, "unable to calculate cmr_div: %d\n",
+				bclk_rate);
+			return bclk_rate;
+		}
+
+		cmr_div = DIV_ROUND_CLOSEST(ssc_p->mck_rate, 2 * bclk_rate);
+	}
+
+	/*
+	 * If the cpu dai should provide LRCLK, but noone has provided the
+	 * dividers needed for that to work, fall back to something sensible.
+	 */
+	tcmr_period = ssc_p->tcmr_period;
+	rcmr_period = ssc_p->rcmr_period;
+	if (atmel_ssc_cfs(ssc_p)) {
+		int frame_size = snd_soc_params_to_frame_size(params);
+
+		if (frame_size < 0) {
+			dev_err(dai->dev,
+				"unable to calculate tx/rx cmr_period: %d\n",
+				frame_size);
+			return frame_size;
+		}
+
+		if (!(ssc_p->forced_divider & BIT(ATMEL_SSC_TCMR_PERIOD)))
+			tcmr_period = frame_size / 2 - 1;
+		if (!(ssc_p->forced_divider & BIT(ATMEL_SSC_RCMR_PERIOD)))
+			rcmr_period = frame_size / 2 - 1;
+	}
+
 	dma_params = ssc_p->dma_params[dir];
 
 	channels = params_channels(params);
@@ -524,7 +593,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 		fslen_ext = (bits - 1) / 16;
 		fslen = (bits - 1) % 16;
 
-		rcmr =	  SSC_BF(RCMR_PERIOD, ssc_p->rcmr_period)
+		rcmr =	  SSC_BF(RCMR_PERIOD, rcmr_period)
 			| SSC_BF(RCMR_STTDLY, START_DELAY)
 			| SSC_BF(RCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(RCMR_CKI, SSC_CKI_RISING)
@@ -540,7 +609,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 			| SSC_BF(RFMR_LOOP, 0)
 			| SSC_BF(RFMR_DATLEN, (bits - 1));
 
-		tcmr =	  SSC_BF(TCMR_PERIOD, ssc_p->tcmr_period)
+		tcmr =	  SSC_BF(TCMR_PERIOD, tcmr_period)
 			| SSC_BF(TCMR_STTDLY, START_DELAY)
 			| SSC_BF(TCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
@@ -606,7 +675,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 		fslen_ext = (bits - 1) / 16;
 		fslen = (bits - 1) % 16;
 
-		rcmr =	  SSC_BF(RCMR_PERIOD, ssc_p->rcmr_period)
+		rcmr =	  SSC_BF(RCMR_PERIOD, rcmr_period)
 			| SSC_BF(RCMR_STTDLY, START_DELAY)
 			| SSC_BF(RCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(RCMR_CKI, SSC_CKI_RISING)
@@ -623,7 +692,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 			| SSC_BF(RFMR_LOOP, 0)
 			| SSC_BF(RFMR_DATLEN, (bits - 1));
 
-		tcmr =	  SSC_BF(TCMR_PERIOD, ssc_p->tcmr_period)
+		tcmr =	  SSC_BF(TCMR_PERIOD, tcmr_period)
 			| SSC_BF(TCMR_STTDLY, START_DELAY)
 			| SSC_BF(TCMR_START, SSC_START_FALLING_RF)
 			| SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
@@ -650,7 +719,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 		 * MCK divider, and the BCLK signal is output
 		 * on the SSC TK line.
 		 */
-		rcmr =	  SSC_BF(RCMR_PERIOD, ssc_p->rcmr_period)
+		rcmr =	  SSC_BF(RCMR_PERIOD, rcmr_period)
 			| SSC_BF(RCMR_STTDLY, 1)
 			| SSC_BF(RCMR_START, SSC_START_RISING_RF)
 			| SSC_BF(RCMR_CKI, SSC_CKI_RISING)
@@ -665,7 +734,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 			| SSC_BF(RFMR_LOOP, 0)
 			| SSC_BF(RFMR_DATLEN, (bits - 1));
 
-		tcmr =	  SSC_BF(TCMR_PERIOD, ssc_p->tcmr_period)
+		tcmr =	  SSC_BF(TCMR_PERIOD, tcmr_period)
 			| SSC_BF(TCMR_STTDLY, 1)
 			| SSC_BF(TCMR_START, SSC_START_RISING_RF)
 			| SSC_BF(TCMR_CKI, SSC_CKI_FALLING)
@@ -760,7 +829,7 @@ static int atmel_ssc_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* set SSC clock mode register */
-	ssc_writel(ssc_p->ssc->regs, CMR, ssc_p->cmr_div);
+	ssc_writel(ssc_p->ssc->regs, CMR, cmr_div);
 
 	/* set receive clock mode and format */
 	ssc_writel(ssc_p->ssc->regs, RCMR, rcmr);
diff --git a/sound/soc/atmel/atmel_ssc_dai.h b/sound/soc/atmel/atmel_ssc_dai.h
index 80b153857a88..75194f582131 100644
--- a/sound/soc/atmel/atmel_ssc_dai.h
+++ b/sound/soc/atmel/atmel_ssc_dai.h
@@ -113,6 +113,7 @@ struct atmel_ssc_info {
 	unsigned short cmr_div;
 	unsigned short tcmr_period;
 	unsigned short rcmr_period;
+	unsigned int forced_divider;
 	struct atmel_pcm_dma_params *dma_params[2];
 	struct atmel_ssc_state ssc_state;
 	unsigned long mck_rate;
-- 
2.1.4

--
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 v3 2/3] ASoC: tse850: document axentia,tse850-pcm5142 bindings
From: Peter Rosin @ 2016-11-11 11:29 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1478863752-10569-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

The TSE-850 is an FM Transmitter Station Equipment, designed to generate
baseband signals for FM, mainly the DARC subcarrier, but other signals
are also possible.

Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 .../bindings/sound/axentia,tse850-pcm5142.txt      | 88 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++
 2 files changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt

diff --git a/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt b/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
new file mode 100644
index 000000000000..0c2d44fda17e
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/axentia,tse850-pcm5142.txt
@@ -0,0 +1,88 @@
+ASoC driver for the Axentia TSE-850 with a PCM5142 codec
+
+Required properties:
+  - compatible: "axentia,tse850-pcm5142"
+  - axentia,ssc-controller: The phandle of the atmel SSC controller used as
+    cpu dai.
+  - axentia,audio-codec: The phandle of the PCM5142 codec.
+  - axentia,add-gpios: gpio specifier that controls the mixer.
+  - axentia,loop1-gpios: gpio specifier that controls loop relays on channel 1.
+  - axentia,loop2-gpios: gpio specifier that controls loop relays on channel 2.
+  - axentia,ana-supply: Regulator that supplies the output amplifier. Must
+    support voltages in the 2V - 20V range, in 1V steps.
+
+The schematics explaining the gpios are as follows:
+
+               loop1 relays
+   IN1 +---o  +------------+  o---+ OUT1
+            \                /
+             +              +
+             |   /          |
+             +--o  +--.     |
+             |  add   |     |
+             |        V     |
+             |      .---.   |
+   DAC +----------->|Sum|---+
+             |      '---'   |
+             |              |
+             +              +
+
+   IN2 +---o--+------------+--o---+ OUT2
+               loop2 relays
+
+The 'loop1' gpio pin controlls two relays, which are either in loop position,
+meaning that input and output are directly connected, or they are in mixer
+position, meaning that the signal is passed through the 'Sum' mixer. Similarly
+for 'loop2'.
+
+In the above, the 'loop1' relays are inactive, thus feeding IN1 to the mixer
+(if 'add' is active) and feeding the mixer output to OUT1. The 'loop2' relays
+are active, short-cutting the TSE-850 from channel 2. IN1, IN2, OUT1 and OUT2
+are TSE-850 connectors and DAC is the PCB name of the (filtered) output from
+the PCM5142 codec.
+
+Example:
+
+	&i2c {
+		codec: pcm5142@4c {
+			compatible = "ti,pcm5142";
+
+			reg = <0x4c>;
+
+			AVDD-supply = <&reg_3v3>;
+			DVDD-supply = <&reg_3v3>;
+			CPVDD-supply = <&reg_3v3>;
+
+			clocks = <&sck>;
+
+			pll-in = <3>;
+			pll-out = <6>;
+		};
+	};
+
+	ana: ana-reg {
+		compatible = "pwm-regulator";
+
+		regulator-name = "ANA";
+
+		pwms = <&pwm0 2 1000 PWM_POLARITY_INVERTED>;
+		pwm-dutycycle-unit = <1000>;
+		pwm-dutycycle-range = <100 1000>;
+
+		regulator-min-microvolt = <2000000>;
+		regulator-max-microvolt = <20000000>;
+		regulator-ramp-delay = <1000>;
+	};
+
+	sound {
+		compatible = "axentia,tse850-pcm5142";
+
+		axentia,ssc-controller = <&ssc0>;
+		axentia,audio-codec = <&codec>;
+
+		axentia,add-gpios = <&pioA 8 GPIO_ACTIVE_LOW>;
+		axentia,loop1-gpios = <&pioA 10 GPIO_ACTIVE_LOW>;
+		axentia,loop2-gpios = <&pioA 11 GPIO_ACTIVE_LOW>;
+
+		axentia,ana-supply = <&ana>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 539b20baf791..4f2ebf3ab51a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2318,6 +2318,12 @@ F:	include/uapi/linux/ax25.h
 F:	include/net/ax25.h
 F:	net/ax25/
 
+AXENTIA ASOC DRIVERS
+M:	Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
+L:	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/sound/axentia,*
+
 AZ6007 DVB DRIVER
 M:	Mauro Carvalho Chehab <mchehab-JsYNTwtnfakRB7SZvlqPiA@public.gmane.org>
 M:	Mauro Carvalho Chehab <mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
-- 
2.1.4

--
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 v3 3/3] ASoC: atmel: tse850: add ASoC driver for the Axentia TSE-850
From: Peter Rosin @ 2016-11-11 11:29 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Nicolas Ferre, Jaroslav Kysela, Takashi Iwai, alsa-devel,
	devicetree
In-Reply-To: <1478863752-10569-1-git-send-email-peda@axentia.se>

The TSE-850 is an FM Transmitter Station Equipment, designed to generate
baseband signals for FM, mainly the DARC subcarrier, but other signals
are also possible.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 MAINTAINERS                      |   1 +
 sound/soc/atmel/Kconfig          |  10 +
 sound/soc/atmel/Makefile         |   2 +
 sound/soc/atmel/tse850-pcm5142.c | 472 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 485 insertions(+)
 create mode 100644 sound/soc/atmel/tse850-pcm5142.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f2ebf3ab51a..4ce45f02d83f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2323,6 +2323,7 @@ M:	Peter Rosin <peda@axentia.se>
 L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/sound/axentia,*
+F:	sound/soc/atmel/tse850-pcm5142.c
 
 AZ6007 DVB DRIVER
 M:	Mauro Carvalho Chehab <mchehab@s-opensource.com>
diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig
index 22aec9a1e9a4..4a56f3dfba51 100644
--- a/sound/soc/atmel/Kconfig
+++ b/sound/soc/atmel/Kconfig
@@ -78,4 +78,14 @@ config SND_ATMEL_SOC_PDMIC
 	help
 	  Say Y if you want to add support for Atmel ASoC driver for boards using
 	  PDMIC.
+
+config SND_ATMEL_SOC_TSE850_PCM5142
+	tristate "ASoC driver for the Axentia TSE-850"
+	depends on ARCH_AT91 && OF
+	depends on ATMEL_SSC && I2C
+	select SND_ATMEL_SOC_SSC_DMA
+	select SND_SOC_PCM512x_I2C
+	help
+	  Say Y if you want to add support for the ASoC driver for the
+	  Axentia TSE-850 with a PCM5142 codec.
 endif
diff --git a/sound/soc/atmel/Makefile b/sound/soc/atmel/Makefile
index a2b127bd9c87..67e10cbd4ed7 100644
--- a/sound/soc/atmel/Makefile
+++ b/sound/soc/atmel/Makefile
@@ -13,9 +13,11 @@ snd-atmel-soc-wm8904-objs := atmel_wm8904.o
 snd-soc-sam9x5-wm8731-objs := sam9x5_wm8731.o
 snd-atmel-soc-classd-objs := atmel-classd.o
 snd-atmel-soc-pdmic-objs := atmel-pdmic.o
+snd-atmel-soc-tse850-pcm5142-objs := tse850-pcm5142.o
 
 obj-$(CONFIG_SND_AT91_SOC_SAM9G20_WM8731) += snd-soc-sam9g20-wm8731.o
 obj-$(CONFIG_SND_ATMEL_SOC_WM8904) += snd-atmel-soc-wm8904.o
 obj-$(CONFIG_SND_AT91_SOC_SAM9X5_WM8731) += snd-soc-sam9x5-wm8731.o
 obj-$(CONFIG_SND_ATMEL_SOC_CLASSD) += snd-atmel-soc-classd.o
 obj-$(CONFIG_SND_ATMEL_SOC_PDMIC) += snd-atmel-soc-pdmic.o
+obj-$(CONFIG_SND_ATMEL_SOC_TSE850_PCM5142) += snd-atmel-soc-tse850-pcm5142.o
diff --git a/sound/soc/atmel/tse850-pcm5142.c b/sound/soc/atmel/tse850-pcm5142.c
new file mode 100644
index 000000000000..ac6a814c8ecf
--- /dev/null
+++ b/sound/soc/atmel/tse850-pcm5142.c
@@ -0,0 +1,472 @@
+/*
+ * TSE-850 audio - ASoC driver for the Axentia TSE-850 with a PCM5142 codec
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * 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.
+ */
+
+/*
+ *               loop1 relays
+ *   IN1 +---o  +------------+  o---+ OUT1
+ *            \                /
+ *             +              +
+ *             |   /          |
+ *             +--o  +--.     |
+ *             |  add   |     |
+ *             |        V     |
+ *             |      .---.   |
+ *   DAC +----------->|Sum|---+
+ *             |      '---'   |
+ *             |              |
+ *             +              +
+ *
+ *   IN2 +---o--+------------+--o---+ OUT2
+ *               loop2 relays
+ *
+ * The 'loop1' gpio pin controlls two relays, which are either in loop
+ * position, meaning that input and output are directly connected, or
+ * they are in mixer position, meaning that the signal is passed through
+ * the 'Sum' mixer. Similarly for 'loop2'.
+ *
+ * In the above, the 'loop1' relays are inactive, thus feeding IN1 to the
+ * mixer (if 'add' is active) and feeding the mixer output to OUT1. The
+ * 'loop2' relays are active, short-cutting the TSE-850 from channel 2.
+ * IN1, IN2, OUT1 and OUT2 are TSE-850 connectors and DAC is the PCB name
+ * of the (filtered) output from the PCM5142 codec.
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+
+#include <sound/soc.h>
+#include <sound/pcm_params.h>
+
+#include "atmel_ssc_dai.h"
+
+struct tse850_priv {
+	int ssc_id;
+
+	struct gpio_desc *add;
+	struct gpio_desc *loop1;
+	struct gpio_desc *loop2;
+
+	struct regulator *ana;
+
+	int add_cache;
+	int loop1_cache;
+	int loop2_cache;
+};
+
+static int tse850_get_mux1(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	ucontrol->value.enumerated.item[0] = tse850->loop1_cache;
+
+	return 0;
+}
+
+static int tse850_put_mux1(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int val = ucontrol->value.enumerated.item[0];
+
+	if (val >= e->items)
+		return -EINVAL;
+
+	gpiod_set_value_cansleep(tse850->loop1, val);
+	tse850->loop1_cache = val;
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+static int tse850_get_mux2(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	ucontrol->value.enumerated.item[0] = tse850->loop2_cache;
+
+	return 0;
+}
+
+static int tse850_put_mux2(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int val = ucontrol->value.enumerated.item[0];
+
+	if (val >= e->items)
+		return -EINVAL;
+
+	gpiod_set_value_cansleep(tse850->loop2, val);
+	tse850->loop2_cache = val;
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+int tse850_get_mix(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	ucontrol->value.enumerated.item[0] = tse850->add_cache;
+
+	return 0;
+}
+
+int tse850_put_mix(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int connect = !!ucontrol->value.integer.value[0];
+
+	if (tse850->add_cache == connect)
+		return 0;
+
+	/*
+	 * Hmmm, this gpiod_set_value_cansleep call should probably happen
+	 * inside snd_soc_dapm_mixer_update_power in the loop.
+	 */
+	gpiod_set_value_cansleep(tse850->add, connect);
+	tse850->add_cache = connect;
+
+	snd_soc_dapm_mixer_update_power(dapm, kctrl, connect, NULL);
+	return 1;
+}
+
+int tse850_get_ana(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	ret = regulator_get_voltage(tse850->ana);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Map regulator output values like so:
+	 *      -11.5V to "Low" (enum 0)
+	 * 11.5V-12.5V to "12V" (enum 1)
+	 * 12.5V-13.5V to "13V" (enum 2)
+	 *     ...
+	 * 18.5V-19.5V to "19V" (enum 8)
+	 * 19.5V-      to "20V" (enum 9)
+	 */
+	if (ret < 11000000)
+		ret = 11000000;
+	else if (ret > 20000000)
+		ret = 20000000;
+	ret -= 11000000;
+	ret = (ret + 500000) / 1000000;
+
+	ucontrol->value.enumerated.item[0] = ret;
+
+	return 0;
+}
+
+int tse850_put_ana(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int uV = ucontrol->value.enumerated.item[0];
+	int ret;
+
+	if (uV >= e->items)
+		return -EINVAL;
+
+	/*
+	 * Map enum zero (Low) to 2 volts on the regulator, do this since
+	 * the ana regulator is supplied by the system 12V voltage and
+	 * requesting anything below the system voltage causes the system
+	 * voltage to be passed through the regulator. Also, the ana
+	 * regulator induces noise when requesting voltages near the
+	 * system voltage. So, by mapping Low to 2V, that noise is
+	 * eliminated when all that is needed is 12V (the system voltage).
+	 */
+	if (uV)
+		uV = 11000000 + (1000000 * uV);
+	else
+		uV = 2000000;
+
+	ret = regulator_set_voltage(tse850->ana, uV, uV);
+	if (ret < 0)
+		return ret;
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+static const char * const mux_text[] = { "Mixer", "Loop" };
+
+static const struct soc_enum mux_enum =
+	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, 2, mux_text);
+
+static const struct snd_kcontrol_new mux1 =
+	SOC_DAPM_ENUM_EXT("MUX1", mux_enum, tse850_get_mux1, tse850_put_mux1);
+
+static const struct snd_kcontrol_new mux2 =
+	SOC_DAPM_ENUM_EXT("MUX2", mux_enum, tse850_get_mux2, tse850_put_mux2);
+
+#define TSE850_DAPM_SINGLE_EXT(xname, reg, shift, max, invert, xget, xput) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_volsw, \
+	.get = xget, \
+	.put = xput, \
+	.private_value = SOC_SINGLE_VALUE(reg, shift, max, invert, 0) }
+
+static const struct snd_kcontrol_new mix[] = {
+	TSE850_DAPM_SINGLE_EXT("IN Switch", SND_SOC_NOPM, 0, 1, 0,
+			       tse850_get_mix, tse850_put_mix),
+};
+
+static const char * const ana_text[] = {
+	"Low", "12V", "13V", "14V", "15V", "16V", "17V", "18V", "19V", "20V"
+};
+
+static const struct soc_enum ana_enum =
+	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, 9, ana_text);
+
+static const struct snd_kcontrol_new out =
+	SOC_DAPM_ENUM_EXT("ANA", ana_enum, tse850_get_ana, tse850_put_ana);
+
+static const struct snd_soc_dapm_widget tse850_dapm_widgets[] = {
+	SND_SOC_DAPM_LINE("OUT1", NULL),
+	SND_SOC_DAPM_LINE("OUT2", NULL),
+	SND_SOC_DAPM_LINE("IN1", NULL),
+	SND_SOC_DAPM_LINE("IN2", NULL),
+	SND_SOC_DAPM_INPUT("DAC"),
+	SND_SOC_DAPM_AIF_IN("AIFINL", "Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("AIFINR", "Playback", 1, SND_SOC_NOPM, 0, 0),
+	SOC_MIXER_ARRAY("MIX", SND_SOC_NOPM, 0, 0, mix),
+	SND_SOC_DAPM_MUX("MUX1", SND_SOC_NOPM, 0, 0, &mux1),
+	SND_SOC_DAPM_MUX("MUX2", SND_SOC_NOPM, 0, 0, &mux2),
+	SND_SOC_DAPM_OUT_DRV("OUT", SND_SOC_NOPM, 0, 0, &out, 1),
+};
+
+/*
+ * These connections are not entirely correct, since both IN1 and IN2
+ * are always fed to MIX (if the "IN switch" is set so), i.e. without
+ * regard to the loop1 and loop2 relays that according to this only
+ * control MUX1 and MUX2 but in fact also control how the input signals
+ * are routed.
+ * But, 1) I don't know how to do it right, and 2) it doesn't seem to
+ * matter in practice since nothing is powered in those sections anyway.
+ */
+static const struct snd_soc_dapm_route tse850_intercon[] = {
+	{ "OUT1", NULL, "MUX1" },
+	{ "OUT2", NULL, "MUX2" },
+
+	{ "MUX1", "Loop",  "IN1" },
+	{ "MUX1", "Mixer", "OUT" },
+
+	{ "MUX2", "Loop",  "IN2" },
+	{ "MUX2", "Mixer", "OUT" },
+
+	{ "OUT", NULL, "MIX" },
+
+	{ "MIX", NULL, "DAC" },
+	{ "MIX", "IN Switch", "IN1" },
+	{ "MIX", "IN Switch", "IN2" },
+
+	/* connect board input to the codec left channel output pin */
+	{ "DAC", NULL, "OUTL" },
+};
+
+static struct snd_soc_dai_link tse850_dailink = {
+	.name = "TSE-850",
+	.stream_name = "TSE-850-PCM",
+	.codec_dai_name = "pcm512x-hifi",
+	.dai_fmt = SND_SOC_DAIFMT_I2S
+		 | SND_SOC_DAIFMT_NB_NF
+		 | SND_SOC_DAIFMT_CBM_CFS,
+};
+
+static struct snd_soc_card tse850_card = {
+	.name = "TSE-850-ASoC",
+	.owner = THIS_MODULE,
+	.dai_link = &tse850_dailink,
+	.num_links = 1,
+	.dapm_widgets = tse850_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(tse850_dapm_widgets),
+	.dapm_routes = tse850_intercon,
+	.num_dapm_routes = ARRAY_SIZE(tse850_intercon),
+	.fully_routed = true,
+};
+
+static int tse850_dt_init(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *codec_np, *cpu_np;
+	struct snd_soc_card *card = &tse850_card;
+	struct snd_soc_dai_link *dailink = &tse850_dailink;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	if (!np) {
+		dev_err(&pdev->dev, "only device tree supported\n");
+		return -EINVAL;
+	}
+
+	cpu_np = of_parse_phandle(np, "axentia,ssc-controller", 0);
+	if (!cpu_np) {
+		dev_err(&pdev->dev, "failed to get dai and pcm info\n");
+		return -EINVAL;
+	}
+	dailink->cpu_of_node = cpu_np;
+	dailink->platform_of_node = cpu_np;
+	tse850->ssc_id = of_alias_get_id(cpu_np, "ssc");
+	of_node_put(cpu_np);
+
+	codec_np = of_parse_phandle(np, "axentia,audio-codec", 0);
+	if (!codec_np) {
+		dev_err(&pdev->dev, "failed to get codec info\n");
+		return -EINVAL;
+	}
+	dailink->codec_of_node = codec_np;
+	of_node_put(codec_np);
+
+	return 0;
+}
+
+static int tse850_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &tse850_card;
+	struct device *dev = card->dev = &pdev->dev;
+	struct tse850_priv *tse850;
+	int ret;
+
+	tse850 = devm_kzalloc(dev, sizeof(*tse850), GFP_KERNEL);
+	if (!tse850)
+		return -ENOMEM;
+
+	snd_soc_card_set_drvdata(card, tse850);
+
+	ret = tse850_dt_init(pdev);
+	if (ret) {
+		dev_err(dev, "failed to init dt info\n");
+		return ret;
+	}
+
+	tse850->add = devm_gpiod_get(dev, "axentia,add", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->add)) {
+		if (PTR_ERR(tse850->add) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'add' gpio\n");
+		return PTR_ERR(tse850->add);
+	}
+	tse850->add_cache = 1;
+
+	tse850->loop1 = devm_gpiod_get(dev, "axentia,loop1", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->loop1)) {
+		if (PTR_ERR(tse850->loop1) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'loop1' gpio\n");
+		return PTR_ERR(tse850->loop1);
+	}
+	tse850->loop1_cache = 1;
+
+	tse850->loop2 = devm_gpiod_get(dev, "axentia,loop2", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->loop2)) {
+		if (PTR_ERR(tse850->loop2) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'loop2' gpio\n");
+		return PTR_ERR(tse850->loop2);
+	}
+	tse850->loop2_cache = 1;
+
+	tse850->ana = devm_regulator_get(dev, "axentia,ana");
+	if (IS_ERR(tse850->ana)) {
+		if (PTR_ERR(tse850->ana) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'ana' regulator\n");
+		return PTR_ERR(tse850->ana);
+	}
+
+	ret = regulator_enable(tse850->ana);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable the 'ana' regulator\n");
+		return ret;
+	}
+
+	ret = atmel_ssc_set_audio(tse850->ssc_id);
+	if (ret != 0) {
+		dev_err(dev,
+			"failed to set SSC %d for audio\n", tse850->ssc_id);
+		goto err_disable_ana;
+	}
+
+	ret = snd_soc_register_card(card);
+	if (ret) {
+		dev_err(dev, "snd_soc_register_card failed\n");
+		goto err_put_audio;
+	}
+
+	return 0;
+
+err_put_audio:
+	atmel_ssc_put_audio(tse850->ssc_id);
+err_disable_ana:
+	regulator_disable(tse850->ana);
+	return ret;
+}
+
+static int tse850_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	snd_soc_unregister_card(card);
+	atmel_ssc_put_audio(tse850->ssc_id);
+	regulator_disable(tse850->ana);
+
+	return 0;
+}
+
+static const struct of_device_id tse850_dt_ids[] = {
+	{ .compatible = "axentia,tse850-pcm5142", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tse850_dt_ids);
+
+static struct platform_driver tse850_driver = {
+	.driver = {
+		.name = "axentia-tse850-pcm5142",
+		.of_match_table = of_match_ptr(tse850_dt_ids),
+	},
+	.probe = tse850_probe,
+	.remove = tse850_remove,
+};
+
+module_platform_driver(tse850_driver);
+
+/* Module information */
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_DESCRIPTION("ALSA SoC driver for TSE-850 with PCM5142 codec");
+MODULE_LICENSE("GPL");
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator
From: Peter Rosin @ 2016-11-11 11:37 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, Daniel Baluta, Slawomir Stepien, linux-iio,
	devicetree
In-Reply-To: <alpine.DEB.2.20.1611091605090.3501@nanos>

On 2016-11-09 16:06, Thomas Gleixner wrote:
> On Wed, 9 Nov 2016, Peter Rosin wrote:
>> On 2016-11-08 22:47, Thomas Gleixner wrote:
>>> I don't think you need extra race handling with that, but I might be wrong
>>> as usual.
>>
>> There's obviously no way to determine which of the timeout or the
>> interrupt that happens first without some race handling, so I don't
>> know what you mean? If the timeout happens first, there is also a
>> need to handle late hits from the irq that might come in during the
>> preparation for the next step in the binary search. It gets messy
>> quickly compared to the simplicity of the current implementation.
> 
> Gah, forgot about that timeout thingy. Fair enough.
> 
> Feel free to add an 
> 
> Acked-by: Thomas Gleixner <tglx@linutronix.de>

Thanks for looking!

Cheers,
Peter

^ permalink raw reply

* Re: [PATCH 2/5] media: i2c: max2175: Add MAX2175 support
From: Laurent Pinchart @ 2016-11-11 11:50 UTC (permalink / raw)
  To: Antti Palosaari
  Cc: Ramesh Shanmugasundaram, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, mchehab-DgEjT+Ai2ygdnm+yROfE0A,
	hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
	sakari.ailus-VuQAYsv1563Yd54FQh9/CA,
	chris.paterson2-zM6kxYcvzFBBDgjK7y7TUQ,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d9e9aac2-001c-491b-db34-a9eed6240722-X3B1VOXEql0@public.gmane.org>

Hi Antti,

On Friday 11 Nov 2016 09:16:04 Antti Palosaari wrote:
> Hello
> 
> On 11/09/2016 05:44 PM, Ramesh Shanmugasundaram wrote:
> > +static int max2175_set_lo_freq(struct max2175 *ctx, u64 lo_freq)
> > +{
> > +	u64 scaled_lo_freq, scaled_npf, scaled_integer, scaled_fraction;
> > +	u32 frac_desired, int_desired, lo_mult = 1;
> > +	const u32 scale_factor = 1000000U;
> > +	u8 loband_bits = 0, vcodiv_bits = 0;
> > +	enum max2175_band band;
> > +	int ret;
> > +
> > +	/* Scale to larger number for precision */
> > +	scaled_lo_freq = lo_freq * scale_factor * 100;
> > +	band = max2175_read_bits(ctx, 5, 1, 0);
> > +
> > +	mxm_dbg(ctx, "set_lo_freq: scaled lo_freq %llu lo_freq %llu band 
%d\n",
> > +		scaled_lo_freq, lo_freq, band);
> > +
> > +	switch (band) {
> > +	case MAX2175_BAND_AM:
> > +		if (max2175_read_bit(ctx, 5, 7) == 0)
> > +			lo_mult = 16;
> 
> else is lo_mult = 1. No idea if it is correct, but sounds very small
> output divider for low freq like am band. And on the other-hand local
> oscillator output divider, which I expect this to be, is usually 2 or more.
> 
> > +		break;
> > +	case MAX2175_BAND_FM:
> > +		if (lo_freq <= 74700000) {
> > +			lo_mult = 16;
> 
> No meaning as you set it later 8.
> 
> > +		} else if (lo_freq > 74700000 && lo_freq <= 110000000) {
> > +			loband_bits = 1;
> > +		} else {
> > +			loband_bits = 1;
> > +			vcodiv_bits = 3;
> > +		}
> > +		lo_mult = 8;
> > +		break;
> > +	case MAX2175_BAND_VHF:
> > +		if (lo_freq <= 210000000) {
> > +			loband_bits = 2;
> > +			vcodiv_bits = 2;
> > +		} else {
> > +			loband_bits = 2;
> > +			vcodiv_bits = 1;
> > +		}
> > +		lo_mult = 4;
> > +		break;
> > +	default:
> > +		loband_bits = 3;
> > +		vcodiv_bits = 2;
> > +		lo_mult = 2;
> > +		break;
> > +	}
> > +
> > +	if (band == MAX2175_BAND_L)
> > +		scaled_npf = div_u64(div_u64(scaled_lo_freq, ctx->xtal_freq),
> > +				     lo_mult);
> > +	else
> > +		scaled_npf = div_u64(scaled_lo_freq, ctx->xtal_freq) * 
lo_mult;
> > +
> > +	scaled_npf = div_u64(scaled_npf, 100);
> > +	scaled_integer = div_u64(scaled_npf, scale_factor) * scale_factor;
> > +	int_desired = div_u64(scaled_npf, scale_factor);
> > +	scaled_fraction = scaled_npf - scaled_integer;
> > +	frac_desired = div_u64(scaled_fraction << 20, scale_factor);
> > +
> > +	/* Check CSM is not busy */
> > +	ret = max2175_poll_csm_ready(ctx);
> > +	if (ret)
> > +		return ret;
> > +
> > +	mxm_dbg(ctx, "loband %u vcodiv %u lo_mult %u scaled_npf %llu\n",
> > +		loband_bits, vcodiv_bits, lo_mult, scaled_npf);
> > +	mxm_dbg(ctx, "scaled int %llu frac %llu desired int %u frac %u\n",
> > +		scaled_integer, scaled_fraction, int_desired, frac_desired);
> > +
> > +	/* Write the calculated values to the appropriate registers */
> > +	max2175_write(ctx, 1, int_desired);
> > +	max2175_write_bits(ctx, 2, 3, 0, (frac_desired >> 16) & 0xf);
> > +	max2175_write(ctx, 3, frac_desired >> 8);
> > +	max2175_write(ctx, 4, frac_desired);
> > +	max2175_write_bits(ctx, 5, 3, 2, loband_bits);
> > +	max2175_write_bits(ctx, 6, 7, 6, vcodiv_bits);
> > +	return ret;
> > +}
> 
> That synthesizer config is hard to understand. It seems to be
> fractional-N, with configurable N, K and output divider - like a school
> book example.
> 
>                +----------------------------+
>                v                            |
>       Fref   +----+     +-------+         +------+
>      ------> | PD | --> |  VCO  | ------> | /N.F |
>              +----+     +-------+         +------+
>                           |
>                           |
>                           v
>                         +-------+  Fout
>                         | /Rout | ------>
>                         +-------+
>
> I made following look-up table in order to understand it:
> 
> band      lo freq band vcodiv div_out
>    AM  <  50000000    0      0      16 // reg 5 bit 7 ?
>    FM  <  74700000    0      0      16
>    FM  < 110000000    1      0       8
>    FM  < 160000000    1      3       8
>   VHF  < 210000000    2      2       4
>   VHF  < 600000000    2      1       4
>     L  <2000000000    3      2       2
> 
> "vcodiv" looks unrelated to synth calculation, dunno what it is.
> 
> One which makes calculation very complex looking is that it is based of
> floating point calculus. On integer mathematics you should replace
> fractional part with fractional modulus (usually letter "K" is used for
> fractional modulus on PLL calc).
> 
> So that ends up something like:
> 1) select suitable lo output divider from desired output frequency
> 2) calculate vco frequency
> 3) convert vco frequency to N and K
> * N = Fvco/Fref
> * K = Fvco%Fref
> 4) convert K to control word (looks like << 20)
> 5) program values
> 
> Result should be calculus without scaling.

Thanks for reviewing this part.

I'd like to add that we already have two PLL helpers in the media subsystem, 
in drivers/media/i2c/aptina-pll.c and drivers/media/i2c/smiapp-pll.c. As the 
PLL used here seems to be a classic one, it would make sense to also extract 
the code in a helper function that could be shared between drivers.

-- 
Regards,

Laurent Pinchart

--
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] PM / Domains: Fix compatible for domain idle state
From: Ulf Hansson @ 2016-11-11 11:52 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lina Iyer, Kevin Hilman, Rafael J. Wysocki,
	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: <20161110195832.4nz7lxlmshaemcbb@rob-hp-laptop>

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.

Yep.

>
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.

Agree!

Kind regards
Uffe
--
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: [RESEND PATCH v1 02/11] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Sysctrl and Djtag dts bindings
From: Mark Rutland @ 2016-11-11 11:53 UTC (permalink / raw)
  To: Anurup M
  Cc: devicetree, linux-arm-kernel, linux-doc, will.deacon, corbet,
	catalin.marinas, robh+dt, arnd, f.fainelli, rmk+kernel, krzk,
	anurup.m, zhangshaokun, tanxiaojun, xuwei5, sanil.kumar,
	john.garry, gabriele.paoloni, shiju.jose, wangkefeng.wang,
	guohanjun, shyju.pv, linuxarm
In-Reply-To: <5825A927.2000202@gmail.com>

On Fri, Nov 11, 2016 at 04:49:03PM +0530, Anurup M wrote:
> On Thursday 10 November 2016 10:53 PM, Mark Rutland wrote:
> >On Thu, Nov 03, 2016 at 01:41:58AM -0400, Anurup M wrote:

> >>diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt

> >>+Example:
> >>+	/* for Hisilicon HiP05 djtag for CPU sysctrl */
> >>+	djtag0: djtag@80010000 {
> >>+		compatible = "hisilicon,hip05-cpu-djtag-v1";
> >>+		reg = <0x0 0x80010000 0x0 0x10000>;
> >>+
> >>+		/* For L3 cache PMU */
> >>+		pmul3c0 {
> >>+			compatible = "hisilicon,hisi-pmu-l3c-v1";
> >>+			scl-id = <0x02>;
> >>+			num-events = <0x16>;
> >>+			num-counters = <0x08>;
> >>+			module-id = <0x04>;
> >>+			num-banks = <0x04>;
> >>+			cfgen-map = <0x02 0x04 0x01 0x08>;
> >>+			counter-reg = <0x170>;
> >>+			evctrl-reg = <0x04>;
> >>+			event-en = <0x1000000>;
> >>+			evtype-reg = <0x140>;
> >>+		};
> >This sub-node needs a binding document.
> >
> >These properties are completely opaque
> The L3 cache PMU bindings are defined @bindings/arm/hisilicon/pmu.txt.
> Is it OK that I document here(hisilicon/djtag.txt), a reference to
> the PMU bindings doc ?

At this point in the series, that file does not exist yet, and this is
an undocumented beinding.

Please introduce this sub-node long with the PMU bindings, later in the
series.

Thanks,
Mark.

^ permalink raw reply

* Re: [RESEND PATCH v1 02/11] dt-bindings: hisi: Add Hisilicon HiP05/06/07 Sysctrl and Djtag dts bindings
From: Anurup M @ 2016-11-11 11:59 UTC (permalink / raw)
  To: Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
	corbet-T1hC0tSOHrs, catalin.marinas-5wv7dgnIgG8,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, arnd-r2nGTMty4D4,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w,
	rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ, krzk-DgEjT+Ai2ygdnm+yROfE0A,
	anurup.m-hv44wF8Li93QT0dZR+AlfA,
	zhangshaokun-C8/M+/jPZTeaMJb+Lgu22Q,
	tanxiaojun-hv44wF8Li93QT0dZR+AlfA, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
	sanil.kumar-C8/M+/jPZTeaMJb+Lgu22Q,
	john.garry-hv44wF8Li93QT0dZR+AlfA,
	gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	shiju.jose-hv44wF8Li93QT0dZR+AlfA,
	wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA,
	guohanjun-hv44wF8Li93QT0dZR+AlfA, shyju.pv-hv44wF8Li93QT0dZR+AlfA,
	linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <20161111115346.GC11945@leverpostej>



On Friday 11 November 2016 05:23 PM, Mark Rutland wrote:
> On Fri, Nov 11, 2016 at 04:49:03PM +0530, Anurup M wrote:
>> On Thursday 10 November 2016 10:53 PM, Mark Rutland wrote:
>>> On Thu, Nov 03, 2016 at 01:41:58AM -0400, Anurup M wrote:
>>>> diff --git a/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt b/Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt
>>>> +Example:
>>>> +	/* for Hisilicon HiP05 djtag for CPU sysctrl */
>>>> +	djtag0: djtag@80010000 {
>>>> +		compatible = "hisilicon,hip05-cpu-djtag-v1";
>>>> +		reg = <0x0 0x80010000 0x0 0x10000>;
>>>> +
>>>> +		/* For L3 cache PMU */
>>>> +		pmul3c0 {
>>>> +			compatible = "hisilicon,hisi-pmu-l3c-v1";
>>>> +			scl-id = <0x02>;
>>>> +			num-events = <0x16>;
>>>> +			num-counters = <0x08>;
>>>> +			module-id = <0x04>;
>>>> +			num-banks = <0x04>;
>>>> +			cfgen-map = <0x02 0x04 0x01 0x08>;
>>>> +			counter-reg = <0x170>;
>>>> +			evctrl-reg = <0x04>;
>>>> +			event-en = <0x1000000>;
>>>> +			evtype-reg = <0x140>;
>>>> +		};
>>> This sub-node needs a binding document.
>>>
>>> These properties are completely opaque
>> The L3 cache PMU bindings are defined @bindings/arm/hisilicon/pmu.txt.
>> Is it OK that I document here(hisilicon/djtag.txt), a reference to
>> the PMU bindings doc ?
> At this point in the series, that file does not exist yet, and this is
> an undocumented beinding.
>
> Please introduce this sub-node long with the PMU bindings, later in the
> series.
Thanks, I got your suggestion. Will add this later in series.
> Thanks,
> Mark.

--
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] media: et8ek8: add device tree binding documentation
From: Pavel Machek @ 2016-11-11 12:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w,
	sakari.ailus-X3B1VOXEql0, sre-DgEjT+Ai2ygdnm+yROfE0A,
	pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
	linux-media-u79uwXL29TY76Z2rM5mHXA, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, mchehab-JPH+aEBZ4P+UEJcrhfAQsw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161107104648.GB5326@amd>

[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]

On Mon 2016-11-07 11:46:48, Pavel Machek wrote:
> Add device tree binding documentation for toshiba et8ek8 sensor.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

Rob, can we get an ack here?

Thanks,
							Pavel

> diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
> new file mode 100644
> index 0000000..b03b21d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/toshiba,et8ek8.txt
> @@ -0,0 +1,53 @@
> +Toshiba et8ek8 5MP sensor
> +
> +Toshiba et8ek8 5MP sensor is an image sensor found in Nokia N900 device
> +
> +More detailed documentation can be found in
> +Documentation/devicetree/bindings/media/video-interfaces.txt .
> +
> +
> +Mandatory properties
> +--------------------
> +
> +- compatible: "toshiba,et8ek8"
> +- reg: I2C address (0x3e, or an alternative address)
> +- vana-supply: Analogue voltage supply (VANA), 2.8 volts
> +- clocks: External clock to the sensor
> +- clock-frequency: Frequency of the external clock to the sensor. Camera
> +  driver will set this frequency on the external clock. The clock frequency is
> +  a pre-determined frequency known to be suitable to the board.
> +- reset-gpios: XSHUTDOWN GPIO. The XSHUTDOWN signal is active high. The sensor
> +  is in hardware standby mode when the signal is in low state.
> +
> +
> +Endpoint node mandatory properties
> +----------------------------------
> +
> +- remote-endpoint: A phandle to the bus receiver's endpoint node.
> +
> +Endpoint node optional properties
> +----------------------------------
> +
> +- clock-lanes: <0>
> +- data-lanes: <1..n>
> +
> +Example
> +-------
> +
> +&i2c3 {
> +	clock-frequency = <400000>;
> +
> +	cam1: camera@3e {
> +		compatible = "toshiba,et8ek8";
> +		reg = <0x3e>;
> +		vana-supply = <&vaux4>;
> +		clocks = <&isp 0>;
> +		clock-frequency = <9600000>;
> +		reset-gpio = <&gpio4 6 GPIO_ACTIVE_HIGH>; /* 102 */
> +		port {
> +			csi_cam1: endpoint {
> +				remote-endpoint = <&csi_out1>;
> +			};
> +		};
> +	};
> +};
> 
> 



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Ulf Hansson @ 2016-11-11 12:34 UTC (permalink / raw)
  To: Dave Gerlach, Rob Herring
  Cc: Nishanth Menon, devicetree@vger.kernel.org, Santosh Shilimkar,
	linux-pm@vger.kernel.org, Kevin Hilman, Sudeep Holla,
	Rafael J . Wysocki, linux-kernel@vger.kernel.org, Jon Hunter,
	Tero Kristo, Russell King, Keerthy,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <90e91588-d2fc-19be-5a28-63c801a8d061@ti.com>

On 10 November 2016 at 20:56, Dave Gerlach <d-gerlach@ti.com> wrote:
> Rob, Ulf, Jon,
>
> On 10/27/2016 08:15 AM, Dave Gerlach wrote:
>>
>> +Jon
>> On 10/26/2016 04:59 PM, Rob Herring wrote:
>>>
>>> On Mon, Oct 24, 2016 at 12:00 PM, Kevin Hilman <khilman@baylibre.com>
>>> wrote:
>>>>
>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>
>>>>> Hi,
>>>>> On 10/21/2016 01:48 PM, Kevin Hilman wrote:
>>>>>>
>>>>>> Dave Gerlach <d-gerlach@ti.com> writes:
>>>>>>
>>>>>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>>>>>> will hook into the genpd framework and allow the TI SCI protocol to
>>>>>>> control device power states.
>>>>>>>
>>>>>>> Also, provide macros representing each device index as understood
>>>>>>> by TI SCI to be used in the device node power-domain references.
>>>>>>> These are identifiers for the K2G devices managed by the PMMC.
>>>>>>>
>>>>>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>>>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/soc/ti/sci-pm-domain.txt   | 54
>>>>>>> +++++++++++++
>>>>>>>  MAINTAINERS                                        |  2 +
>>>>>>>  include/dt-bindings/genpd/k2g.h                    | 90
>>>>>>> ++++++++++++++++++++++
>>>>>>>  3 files changed, 146 insertions(+)
>>>>>>>  create mode 100644
>>>>>>> Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>>  create mode 100644 include/dt-bindings/genpd/k2g.h
>>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..32f38a349656
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>>>>>> @@ -0,0 +1,54 @@
>>>>>>> +Texas Instruments TI-SCI Generic Power Domain
>>>>>>> +---------------------------------------------
>>>>>>> +
>>>>>>> +Some TI SoCs contain a system controller (like the PMMC, etc...)
>>>>>>> that is
>>>>>>> +responsible for controlling the state of the IPs that are present.
>>>>>>> +Communication between the host processor running an OS and the
>>>>>>> system
>>>>>>> +controller happens through a protocol known as TI-SCI [1]. This pm
>>>>>>> domain
>>>>>>> +implementation plugs into the generic pm domain framework and makes
>>>>>>> use of
>>>>>>> +the TI SCI protocol power on and off each device when needed.
>>>>>>> +
>>>>>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>>>>>> +
>>>>>>> +PM Domain Node
>>>>>>> +==============
>>>>>>> +The PM domain node represents the global PM domain managed by the
>>>>>>> PMMC,
>>>>>>> +which in this case is the single implementation as documented by the
>>>>>>> generic
>>>>>>> +PM domain bindings in
>>>>>>> Documentation/devicetree/bindings/power/power_domain.txt.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- compatible: should be "ti,sci-pm-domain"
>>>>>>> +- #power-domain-cells: Must be 0.
>>>>>>> +- ti,sci: Phandle to the TI SCI device to use for managing the
>>>>>>> devices.
>>>>>>>
>>>>>>> +Example:
>>>>>>> +--------------------
>>>>>>> +k2g_pds: k2g_pds {
>>>>>>
>>>>>>
>>>>>> should use generic name like "power-contoller", e.g. k2g_pds:
>>>>>> power-controller
>>>>>
>>>>>
>>>>> Ok, that makes more sense.
>>>>>
>>>>>>
>>>>>>> +        compatible = "ti,sci-pm-domain";
>>>>>>> +        #power-domain-cells = <0>;
>>>>>>> +        ti,sci = <&pmmc>;
>>>>>>> +};
>>>>>>> +
>>>>>>> +PM Domain Consumers
>>>>>>> +===================
>>>>>>> +Hardware blocks that require SCI control over their state must
>>>>>>> provide
>>>>>>> +a reference to the sci-pm-domain they are part of and a unique
>>>>>>> device
>>>>>>> +specific ID that identifies the device.
>>>>>>> +
>>>>>>> +Required Properties:
>>>>>>> +--------------------
>>>>>>> +- power-domains: phandle pointing to the corresponding PM domain
>>>>>>> node.
>>>>>>> +- ti,sci-id: index representing the device id to be passed oevr SCI
>>>>>>> to
>>>>>>> +        be used for device control.
>>>>>>
>>>>>>
>>>>>> This ID doesn't look right.
>>>>>>
>>>>>> Why not use #power-domain-cells = <1> and pass the index in the DT?
>>>>>> ...
>>>
>>>
>>> Exactly. ti,sci-id is a NAK for me.
>>
>>
>> I was told not to use the onecell during v1 discussion. I agree this would
>> be
>> ideal but I cannot due to what the bindings represent, the phandle
>> parameter is
>> an index into a list of genpds, whereas we need an actual ID number we can
>> use
>> and I do not have the ability to get that from the phandle.
>>
>> @Ulf/Jon, is there any hope of bringing back custom xlate functions for
>> genpd
>> providers? I don't have a good background on why it was even removed. I
>> can
>> maintain a single genpd for all devices but I need a way to parse this ID,
>> whether it's from a separate property or a phandle. It is locked now to
>> indexing
>> into a list of genpds but I need additional per device information for
>> devices
>> bound to a genpd and I need either a custom parameter or the ability to
>> parse
>> the phandle myself.
>>
>
> Any comments here? The meaning of the phandle onecell is fixed in the genpd
> framework so I'm not sure how we want to move forward with this, I need to
> pass a power domain ID to the genpd driver, and if this shouldn't be a new
> property I'm not sure what direction we should take.
>
> Regards,
> Dave
>
>
>>>
>>>>>>
>>>>>>> +See dt-bindings/genpd/k2g.h for the list of valid identifiers for
>>>>>>> k2g.
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +--------------------
>>>>>>> +uart0: serial@02530c00 {
>>>>>>> +   compatible = "ns16550a";
>>>>>>> +   ...
>>>>>>> +   power-domains = <&k2g_pds>;
>>>>>>> +   ti,sci-id = <K2G_DEV_UART0>;
>>>>>>
>>>>>>
>>>>>> ... like this:
>>>>>>
>>>>>>      power-domains = <&k2g_pds K2G_DEV_UART0>;
>>>>>
>>>>>
>>>>> That's how I did it in version one actually. I was able to define my
>>>>> own xlate function to parse the phandle and get that index, but Ulf
>>>>> pointed me to this series by Jon Hunter [1] that simplified genpd
>>>>> providers and dropped the concept of adding your own xlate. This locks
>>>>> the onecell approach to using a fixed static array of genpds that get
>>>>> indexed into (without passing the index to the provider, just the
>>>>> genpd that's looked up), which doesn't fit our usecase, as we don't
>>>>> want a 1 to 1 genpd to device mapping based on the comments provided
>>>>> in v1. Now we just use the genpd device attach/detach hooks to parse
>>>>> the sci-id and then use it in the genpd device start/stop hooks.
>>>
>>>
>>> I have no idea what any of this means. All sounds like driver
>>> architecture, not anything to do with bindings.
>>
>>
>> This was a response to Kevin, not part of binding description.
>>
>>>
>>>>
>>>> Ah, right.  I remember now.  This approach allows you to use a single
>>>> genpd as discussed earlier.
>>>>
>>>> Makes sense now, suggestion retracted.
>>>
>>>
>>> IIRC, the bindings in Jon's case had a node for each domain and didn't
>>> need any additional property.
>>
>>
>> Yes but we only have one domain and index into it, not into a list of
>> domains,

Exactly. And this my main point as well. We are not talking about a
domain property but a device property.

>> so the additional property is solving a different problem.

Yes.

Perhaps you could try to elaborate about what the TI SCI ID really
represents for the device, as to help Rob understand the bigger
picture?

To me, the TI SCI ID, is similar to a "conid" for any another "device
resource" (like clock, pinctrl, regulator etc) which we can describe
in DT and assign to a device node. The only difference here, is that
we don't have common API to fetch the resource (like clk_get(),
regulator_get()), but instead we fetches the device's resource from
SoC specific code, via genpd's device ->attach() callback.

Hope that helps.

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH 2/5] media: i2c: max2175: Add MAX2175 support
From: Hans Verkuil @ 2016-11-11 13:21 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram, robh+dt, mark.rutland, mchehab,
	sakari.ailus, crope
  Cc: chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <1478706284-59134-3-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

Hi Ramesh,

A quick review:

On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
> This patch adds driver support for MAX2175 chip. This is Maxim
> Integrated's RF to Bits tuner front end chip designed for software-defined
> radio solutions. This driver exposes the tuner as a sub-device instance
> with standard and custom controls to configure the device.
> 
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> ---
>  .../devicetree/bindings/media/i2c/max2175.txt      |   61 +
>  drivers/media/i2c/Kconfig                          |    4 +
>  drivers/media/i2c/Makefile                         |    2 +
>  drivers/media/i2c/max2175/Kconfig                  |    8 +
>  drivers/media/i2c/max2175/Makefile                 |    4 +
>  drivers/media/i2c/max2175/max2175.c                | 1558 ++++++++++++++++++++
>  drivers/media/i2c/max2175/max2175.h                |  108 ++
>  7 files changed, 1745 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/max2175.txt
>  create mode 100644 drivers/media/i2c/max2175/Kconfig
>  create mode 100644 drivers/media/i2c/max2175/Makefile
>  create mode 100644 drivers/media/i2c/max2175/max2175.c
>  create mode 100644 drivers/media/i2c/max2175/max2175.h
> 

<snip>

> diff --git a/drivers/media/i2c/max2175/max2175.c b/drivers/media/i2c/max2175/max2175.c
> new file mode 100644
> index 0000000..ec45b52
> --- /dev/null
> +++ b/drivers/media/i2c/max2175/max2175.c
> @@ -0,0 +1,1558 @@

<snip>

> +/* Read/Write bit(s) on top of regmap */
> +static int max2175_read(struct max2175 *ctx, u8 idx, u8 *val)
> +{
> +	u32 regval;
> +	int ret = regmap_read(ctx->regmap, idx, &regval);
> +
> +	if (ret)
> +		v4l2_err(ctx->client, "read ret(%d): idx 0x%02x\n", ret, idx);
> +
> +	*val = regval;

Does regmap_read initialize regval even if it returns an error? If not,
then I would initialize regval to 0 to prevent *val being uninitialized.

> +	return ret;
> +}
> +
> +static int max2175_write(struct max2175 *ctx, u8 idx, u8 val)
> +{
> +	int ret = regmap_write(ctx->regmap, idx, val);
> +
> +	if (ret)
> +		v4l2_err(ctx->client, "write ret(%d): idx 0x%02x val 0x%02x\n",
> +			 ret, idx, val);
> +	return ret;
> +}
> +
> +static u8 max2175_read_bits(struct max2175 *ctx, u8 idx, u8 msb, u8 lsb)
> +{
> +	u8 val;
> +
> +	if (max2175_read(ctx, idx, &val))
> +		return 0;
> +
> +	return max2175_get_bitval(val, msb, lsb);
> +}
> +
> +static bool max2175_read_bit(struct max2175 *ctx, u8 idx, u8 bit)
> +{
> +	return !!max2175_read_bits(ctx, idx, bit, bit);
> +}
> +
> +static int max2175_write_bits(struct max2175 *ctx, u8 idx,
> +			     u8 msb, u8 lsb, u8 newval)
> +{
> +	int ret = regmap_update_bits(ctx->regmap, idx, GENMASK(msb, lsb),
> +				     newval << lsb);
> +
> +	if (ret)
> +		v4l2_err(ctx->client, "wbits ret(%d): idx 0x%02x\n", ret, idx);
> +
> +	return ret;
> +}
> +
> +static int max2175_write_bit(struct max2175 *ctx, u8 idx, u8 bit, u8 newval)
> +{
> +	return max2175_write_bits(ctx, idx, bit, bit, newval);
> +}
> +
> +/* Checks expected pattern every msec until timeout */
> +static int max2175_poll_timeout(struct max2175 *ctx, u8 idx, u8 msb, u8 lsb,
> +				u8 exp_bitval, u32 timeout_ms)
> +{
> +	unsigned int val;
> +
> +	return regmap_read_poll_timeout(ctx->regmap, idx, val,
> +			(max2175_get_bitval(val, msb, lsb) == exp_bitval),
> +			1000, timeout_ms * 1000);
> +}
> +
> +static int max2175_poll_csm_ready(struct max2175 *ctx)
> +{
> +	int ret;
> +
> +	ret = max2175_poll_timeout(ctx, 69, 1, 1, 0, 50);
> +	if (ret)
> +		v4l2_err(ctx->client, "csm not ready\n");
> +
> +	return ret;
> +}
> +
> +#define MAX2175_IS_BAND_AM(ctx)		\
> +	(max2175_read_bits(ctx, 5, 1, 0) == MAX2175_BAND_AM)
> +
> +#define MAX2175_IS_BAND_VHF(ctx)	\
> +	(max2175_read_bits(ctx, 5, 1, 0) == MAX2175_BAND_VHF)
> +
> +#define MAX2175_IS_FM_MODE(ctx)		\
> +	(max2175_read_bits(ctx, 12, 5, 4) == 0)
> +
> +#define MAX2175_IS_FMHD_MODE(ctx)	\
> +	(max2175_read_bits(ctx, 12, 5, 4) == 1)
> +
> +#define MAX2175_IS_DAB_MODE(ctx)	\
> +	(max2175_read_bits(ctx, 12, 5, 4) == 2)
> +
> +static int max2175_band_from_freq(u32 freq)
> +{
> +	if (freq >= 144000 && freq <= 26100000)
> +		return MAX2175_BAND_AM;
> +	else if (freq >= 65000000 && freq <= 108000000)
> +		return MAX2175_BAND_FM;
> +	else

No need for these 'else' keywords.

> +		return MAX2175_BAND_VHF;
> +}
> +
> +static int max2175_update_i2s_mode(struct max2175 *ctx, u32 rx_mode,
> +				   u32 i2s_mode)
> +{
> +	max2175_write_bits(ctx, 29, 2, 0, i2s_mode);
> +
> +	/* Based on I2S mode value I2S_WORD_CNT values change */
> +	switch (i2s_mode) {
> +	case MAX2175_I2S_MODE3:
> +		max2175_write_bits(ctx, 30, 6, 0, 1);
> +		break;
> +	case MAX2175_I2S_MODE2:
> +	case MAX2175_I2S_MODE4:
> +		max2175_write_bits(ctx, 30, 6, 0, 0);
> +		break;
> +	case MAX2175_I2S_MODE0:
> +		max2175_write_bits(ctx, 30, 6, 0,
> +			ctx->rx_modes[rx_mode].i2s_word_size);
> +		break;
> +	}
> +	mxm_dbg(ctx, "update_i2s_mode %u, rx_mode %u\n", i2s_mode, rx_mode);
> +	return 0;
> +}
> +
> +static void max2175_i2s_enable(struct max2175 *ctx, bool enable)
> +{
> +	if (enable) {
> +		/* Use old setting */
> +		max2175_write_bits(ctx, 104, 3, 0, ctx->i2s_test);
> +	} else {
> +		/* Cache old setting */
> +		ctx->i2s_test = max2175_read_bits(ctx, 104, 3, 0);
> +		max2175_write_bits(ctx, 104, 3, 0, 9);	/* Keep SCK alive */
> +	}
> +	mxm_dbg(ctx, "i2s %sabled: old val %u\n", enable ? "en" : "dis",
> +		ctx->i2s_test);
> +}
> +
> +static void max2175_set_filter_coeffs(struct max2175 *ctx, u8 m_sel,
> +				      u8 bank, const u16 *coeffs)
> +{
> +	unsigned int i;
> +	u8 coeff_addr, upper_address = 24;
> +
> +	mxm_dbg(ctx, "set_filter_coeffs: m_sel %d bank %d\n", m_sel, bank);
> +	max2175_write_bits(ctx, 114, 5, 4, m_sel);
> +
> +	if (m_sel == 2)
> +		upper_address = 12;
> +
> +	for (i = 0; i < upper_address; i++) {
> +		coeff_addr = i + bank * 24;
> +		max2175_write(ctx, 115, coeffs[i] >> 8);
> +		max2175_write(ctx, 116, coeffs[i]);
> +		max2175_write(ctx, 117, coeff_addr | 1 << 7);
> +	}
> +	max2175_write_bit(ctx, 117, 7, 0);
> +}
> +
> +static void max2175_load_fmeu_1p2(struct max2175 *ctx)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(fmeu1p2_map); i++)
> +		max2175_write(ctx, fmeu1p2_map[i].idx, fmeu1p2_map[i].val);
> +
> +	ctx->decim_ratio = 36;
> +
> +	/* Load the Channel Filter Coefficients into channel filter bank #2 */
> +	max2175_set_filter_coeffs(ctx, MAX2175_CH_MSEL, 0, ch_coeff_fmeu);
> +	max2175_set_filter_coeffs(ctx, MAX2175_EQ_MSEL, 0,
> +				  eq_coeff_fmeu1_ra02_m6db);
> +}
> +
> +static void max2175_load_dab_1p2(struct max2175 *ctx)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(dab12_map); i++)
> +		max2175_write(ctx, dab12_map[i].idx, dab12_map[i].val);
> +
> +	ctx->decim_ratio = 1;
> +
> +	/* Load the Channel Filter Coefficients into channel filter bank #2 */
> +	max2175_set_filter_coeffs(ctx, MAX2175_CH_MSEL, 2, ch_coeff_dab1);
> +}
> +
> +static void max2175_load_fmna_1p0(struct max2175 *ctx)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(fmna1p0_map); i++)
> +		max2175_write(ctx, fmna1p0_map[i].idx, fmna1p0_map[i].val);
> +}
> +
> +static void max2175_load_fmna_2p0(struct max2175 *ctx)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(fmna2p0_map); i++)
> +		max2175_write(ctx, fmna2p0_map[i].idx, fmna2p0_map[i].val);
> +}
> +
> +static void max2175_set_bbfilter(struct max2175 *ctx)
> +{
> +	if (MAX2175_IS_BAND_AM(ctx)) {
> +		max2175_write_bits(ctx, 12, 3, 0, ctx->rom_bbf_bw_am);
> +		mxm_dbg(ctx, "set_bbfilter AM: rom %d\n", ctx->rom_bbf_bw_am);
> +	} else if (MAX2175_IS_DAB_MODE(ctx)) {
> +		max2175_write_bits(ctx, 12, 3, 0, ctx->rom_bbf_bw_dab);
> +		mxm_dbg(ctx, "set_bbfilter DAB: rom %d\n", ctx->rom_bbf_bw_dab);
> +	} else {
> +		max2175_write_bits(ctx, 12, 3, 0, ctx->rom_bbf_bw_fm);
> +		mxm_dbg(ctx, "set_bbfilter FM: rom %d\n", ctx->rom_bbf_bw_fm);
> +	}
> +}
> +
> +static bool max2175_set_csm_mode(struct max2175 *ctx,
> +			  enum max2175_csm_mode new_mode)
> +{
> +	int ret = max2175_poll_csm_ready(ctx);
> +
> +	if (ret)
> +		return ret;
> +
> +	max2175_write_bits(ctx, 0, 2, 0, new_mode);
> +	mxm_dbg(ctx, "set csm new mode %d\n", new_mode);
> +
> +	/* Wait for a fixed settle down time depending on new mode */
> +	switch (new_mode) {
> +	case MAX2175_PRESET_TUNE:
> +		usleep_range(51100, 51500);	/* 51.1ms */
> +		break;
> +	/*
> +	 * Other mode switches need different sleep values depending on band &
> +	 * mode
> +	 */
> +	default:
> +		break;
> +	}
> +
> +	return max2175_poll_csm_ready(ctx);
> +}
> +
> +static int max2175_csm_action(struct max2175 *ctx,
> +			      enum max2175_csm_mode action)
> +{
> +	int ret;
> +
> +	mxm_dbg(ctx, "csm_action: %d\n", action);
> +
> +	/* Other actions can be added in future when needed */
> +	ret = max2175_set_csm_mode(ctx, MAX2175_LOAD_TO_BUFFER);
> +	if (ret)
> +		return ret;
> +	return max2175_set_csm_mode(ctx, MAX2175_PRESET_TUNE);
> +}
> +
> +static int max2175_set_lo_freq(struct max2175 *ctx, u64 lo_freq)
> +{
> +	u64 scaled_lo_freq, scaled_npf, scaled_integer, scaled_fraction;
> +	u32 frac_desired, int_desired, lo_mult = 1;
> +	const u32 scale_factor = 1000000U;
> +	u8 loband_bits = 0, vcodiv_bits = 0;
> +	enum max2175_band band;
> +	int ret;
> +
> +	/* Scale to larger number for precision */
> +	scaled_lo_freq = lo_freq * scale_factor * 100;
> +	band = max2175_read_bits(ctx, 5, 1, 0);
> +
> +	mxm_dbg(ctx, "set_lo_freq: scaled lo_freq %llu lo_freq %llu band %d\n",
> +		scaled_lo_freq, lo_freq, band);
> +
> +	switch (band) {
> +	case MAX2175_BAND_AM:
> +		if (max2175_read_bit(ctx, 5, 7) == 0)
> +			lo_mult = 16;
> +		break;
> +	case MAX2175_BAND_FM:
> +		if (lo_freq <= 74700000) {
> +			lo_mult = 16;
> +		} else if (lo_freq > 74700000 && lo_freq <= 110000000) {
> +			loband_bits = 1;
> +		} else {
> +			loband_bits = 1;
> +			vcodiv_bits = 3;
> +		}
> +		lo_mult = 8;
> +		break;
> +	case MAX2175_BAND_VHF:
> +		if (lo_freq <= 210000000) {
> +			loband_bits = 2;
> +			vcodiv_bits = 2;
> +		} else {
> +			loband_bits = 2;
> +			vcodiv_bits = 1;
> +		}
> +		lo_mult = 4;
> +		break;
> +	default:
> +		loband_bits = 3;
> +		vcodiv_bits = 2;
> +		lo_mult = 2;
> +		break;
> +	}
> +
> +	if (band == MAX2175_BAND_L)
> +		scaled_npf = div_u64(div_u64(scaled_lo_freq, ctx->xtal_freq),
> +				     lo_mult);
> +	else
> +		scaled_npf = div_u64(scaled_lo_freq, ctx->xtal_freq) * lo_mult;
> +
> +	scaled_npf = div_u64(scaled_npf, 100);
> +	scaled_integer = div_u64(scaled_npf, scale_factor) * scale_factor;
> +	int_desired = div_u64(scaled_npf, scale_factor);
> +	scaled_fraction = scaled_npf - scaled_integer;
> +	frac_desired = div_u64(scaled_fraction << 20, scale_factor);
> +
> +	/* Check CSM is not busy */
> +	ret = max2175_poll_csm_ready(ctx);
> +	if (ret)
> +		return ret;
> +
> +	mxm_dbg(ctx, "loband %u vcodiv %u lo_mult %u scaled_npf %llu\n",
> +		loband_bits, vcodiv_bits, lo_mult, scaled_npf);
> +	mxm_dbg(ctx, "scaled int %llu frac %llu desired int %u frac %u\n",
> +		scaled_integer, scaled_fraction, int_desired, frac_desired);
> +
> +	/* Write the calculated values to the appropriate registers */
> +	max2175_write(ctx, 1, int_desired);
> +	max2175_write_bits(ctx, 2, 3, 0, (frac_desired >> 16) & 0xf);
> +	max2175_write(ctx, 3, frac_desired >> 8);
> +	max2175_write(ctx, 4, frac_desired);
> +	max2175_write_bits(ctx, 5, 3, 2, loband_bits);
> +	max2175_write_bits(ctx, 6, 7, 6, vcodiv_bits);
> +	return ret;
> +}
> +
> +static int max2175_set_nco_freq(struct max2175 *ctx, s64 nco_freq_desired)
> +{
> +	s64  nco_freq, nco_val_desired;
> +	u64 abs_nco_freq;
> +	const u32 scale_factor = 1000000U;
> +	u32 clock_rate, nco_reg;
> +	int ret;
> +
> +	mxm_dbg(ctx, "set_nco_freq: freq %lld\n", nco_freq_desired);
> +	clock_rate = ctx->xtal_freq / ctx->decim_ratio;
> +	nco_freq = -nco_freq_desired;
> +
> +	if (nco_freq < 0)
> +		abs_nco_freq = -nco_freq;
> +	else
> +		abs_nco_freq = nco_freq;
> +
> +	/* Scale up the values for precision */
> +	if (abs_nco_freq < (clock_rate / 2)) {
> +		nco_val_desired = div_s64(2 * nco_freq * scale_factor,
> +					  clock_rate);
> +	} else {
> +		if (nco_freq < 0)
> +			nco_val_desired =
> +			div_s64(-2 * (clock_rate - abs_nco_freq) * scale_factor,
> +				clock_rate);
> +		else
> +			nco_val_desired =
> +			div_s64(2 * (clock_rate - abs_nco_freq) * scale_factor,
> +				clock_rate);
> +	}
> +
> +	/* Scale down to get the fraction */
> +	if (nco_freq < 0)
> +		nco_reg = 0x200000 + div_s64(nco_val_desired << 20,
> +						  scale_factor);
> +	else
> +		nco_reg = div_s64(nco_val_desired << 20, scale_factor);
> +
> +	/* Check CSM is not busy */
> +	ret = max2175_poll_csm_ready(ctx);
> +	if (ret)
> +		return ret;
> +
> +	mxm_dbg(ctx, "clk %u decim %u abs %llu desired %lld reg %u\n",
> +		clock_rate, ctx->decim_ratio, abs_nco_freq,
> +		nco_val_desired, nco_reg);
> +
> +	/* Write the calculated values to the appropriate registers */
> +	max2175_write_bits(ctx, 7, 4, 0, (nco_reg >> 16) & 0x1f);
> +	max2175_write(ctx, 8, nco_reg >> 8);
> +	max2175_write(ctx, 9, nco_reg);
> +	return ret;
> +}
> +
> +static int max2175_set_rf_freq_non_am_bands(struct max2175 *ctx, u64 freq,
> +					    u32 lo_pos)
> +{
> +	s64 adj_freq;
> +	u64 low_if_freq;
> +	int ret;
> +
> +	mxm_dbg(ctx, "rf_freq: non AM bands\n");
> +
> +	if (MAX2175_IS_FM_MODE(ctx))
> +		low_if_freq = 128000;
> +	else if (MAX2175_IS_FMHD_MODE(ctx))
> +		low_if_freq = 228000;
> +	else
> +		return max2175_set_lo_freq(ctx, freq);
> +
> +	if (MAX2175_IS_BAND_VHF(ctx) == (lo_pos == MAX2175_LO_ABOVE_DESIRED))
> +		adj_freq = freq + low_if_freq;
> +	else
> +		adj_freq = freq - low_if_freq;
> +
> +	ret = max2175_set_lo_freq(ctx, adj_freq);
> +	if (ret)
> +		return ret;
> +
> +	return max2175_set_nco_freq(ctx, low_if_freq);
> +}
> +
> +static int max2175_set_rf_freq(struct max2175 *ctx, u64 freq, u32 lo_pos)
> +{
> +	int ret;
> +
> +	if (MAX2175_IS_BAND_AM(ctx))
> +		ret = max2175_set_nco_freq(ctx, freq);
> +	else
> +		ret = max2175_set_rf_freq_non_am_bands(ctx, freq, lo_pos);
> +
> +	mxm_dbg(ctx, "set_rf_freq: ret %d freq %llu\n", ret, freq);
> +	return ret;
> +}
> +
> +static int max2175_tune_rf_freq(struct max2175 *ctx, u64 freq, u32 hsls)
> +{
> +	int ret;
> +
> +	ret = max2175_set_rf_freq(ctx, freq, hsls);
> +	if (ret)
> +		return ret;
> +
> +	ret = max2175_csm_action(ctx, MAX2175_BUFFER_PLUS_PRESET_TUNE);
> +	if (ret)
> +		return ret;
> +
> +	mxm_dbg(ctx, "tune_rf_freq: old %u new %llu\n", ctx->freq, freq);
> +	ctx->freq = freq;
> +	return ret;
> +}
> +
> +static void max2175_set_hsls(struct max2175 *ctx, u32 lo_pos)
> +{
> +	mxm_dbg(ctx, "set_hsls: lo_pos %u\n", lo_pos);
> +
> +	if ((lo_pos == MAX2175_LO_BELOW_DESIRED) == MAX2175_IS_BAND_VHF(ctx))
> +		max2175_write_bit(ctx, 5, 4, 1);
> +	else
> +		max2175_write_bit(ctx, 5, 4, 0);
> +}
> +
> +static void max2175_set_eu_rx_mode(struct max2175 *ctx, u32 rx_mode)
> +{
> +	switch (rx_mode) {
> +	case MAX2175_EU_FM_1_2:
> +		max2175_load_fmeu_1p2(ctx);
> +		break;
> +
> +	case MAX2175_DAB_1_2:
> +		max2175_load_dab_1p2(ctx);
> +		break;
> +	}
> +	/* Master is the default setting */
> +	if (!ctx->master)
> +		max2175_write_bit(ctx, 30, 7, 1);
> +
> +	/* Cache i2s_test value at this point */
> +	ctx->i2s_test = max2175_read_bits(ctx, 104, 3, 0);
> +}
> +
> +static void max2175_set_na_rx_mode(struct max2175 *ctx, u32 rx_mode)
> +{
> +	switch (rx_mode) {
> +	case MAX2175_NA_FM_1_0:
> +		max2175_load_fmna_1p0(ctx);
> +		break;
> +	case MAX2175_NA_FM_2_0:
> +		max2175_load_fmna_2p0(ctx);
> +		break;
> +	}
> +	/* Master is the default setting */
> +	if (!ctx->master)
> +		max2175_write_bit(ctx, 30, 7, 1);
> +
> +	/* Cache i2s_test value at this point */
> +	ctx->i2s_test = max2175_read_bits(ctx, 104, 3, 0);
> +	ctx->decim_ratio = 27;
> +
> +	/* Load the Channel Filter Coefficients into channel filter bank #2 */
> +	max2175_set_filter_coeffs(ctx, MAX2175_CH_MSEL, 0, ch_coeff_fmna);
> +	max2175_set_filter_coeffs(ctx, MAX2175_EQ_MSEL, 0,
> +				  eq_coeff_fmna1_ra02_m6db);
> +}
> +
> +static int max2175_set_rx_mode(struct max2175 *ctx, u32 rx_mode, u32 hsls)
> +{
> +	mxm_dbg(ctx, "set_rx_mode: %u am_hiz %u\n", rx_mode, ctx->am_hiz);
> +	if (ctx->xtal_freq == MAX2175_EU_XTAL_FREQ)
> +		max2175_set_eu_rx_mode(ctx, rx_mode);
> +	else
> +		max2175_set_na_rx_mode(ctx, rx_mode);
> +
> +	if (ctx->am_hiz) {
> +		mxm_dbg(ctx, "setting AM HiZ related config\n");
> +		max2175_write_bit(ctx, 50, 5, 1);
> +		max2175_write_bit(ctx, 90, 7, 1);
> +		max2175_write_bits(ctx, 73, 1, 0, 2);
> +		max2175_write_bits(ctx, 80, 5, 0, 33);
> +	}
> +
> +	/* Load BB filter trim values saved in ROM */
> +	max2175_set_bbfilter(ctx);
> +
> +	/* Set HSLS */
> +	max2175_set_hsls(ctx, hsls);
> +
> +	ctx->mode_resolved = true;
> +	return 0;
> +}
> +
> +static bool max2175_i2s_rx_mode_valid(struct max2175 *ctx,
> +					 u32 mode, u32 i2s_mode)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->rx_modes[mode].i2s_modes); i++)
> +		if (ctx->rx_modes[mode].i2s_modes[i] == i2s_mode)
> +			return true;
> +
> +	v4l2_err(ctx->client, "i2s_mode %u not suitable for cur rx mode %u\n",
> +		 i2s_mode, mode);
> +	return false;
> +}
> +
> +static int max2175_rx_mode_from_freq(struct max2175 *ctx, u32 freq, u32 *mode)
> +{
> +	unsigned int i;
> +	int band = max2175_band_from_freq(freq);
> +
> +	/* Pick the first match always */
> +	for (i = 0; i <= ctx->rx_mode->maximum; i++) {
> +		if (ctx->rx_modes[i].band == band) {
> +			*mode = i;
> +			mxm_dbg(ctx, "rx_mode_from_freq: freq %u mode %d\n",
> +				freq, *mode);
> +			return 0;
> +		}
> +	}
> +	return -EINVAL;
> +}
> +
> +static bool max2175_freq_rx_mode_valid(struct max2175 *ctx,
> +					 u32 mode, u32 freq)
> +{
> +	int band = max2175_band_from_freq(freq);
> +
> +	return (ctx->rx_modes[mode].band == band);
> +}
> +
> +static void max2175_load_adc_presets(struct max2175 *ctx)
> +{
> +	unsigned int i, j;
> +
> +	for (i = 0; i < 2; i++)
> +		for (j = 0; j < 23; j++)
> +			max2175_write(ctx, 146 + j + i * 55, adc_presets[i][j]);
> +}
> +
> +static int max2175_init_power_manager(struct max2175 *ctx)
> +{
> +	int ret;
> +
> +	/* Execute on-chip power-up/calibration */
> +	max2175_write_bit(ctx, 99, 2, 0);
> +	usleep_range(1000, 1500);
> +	max2175_write_bit(ctx, 99, 2, 1);
> +
> +	/* Wait for the power manager to finish. */
> +	ret = max2175_poll_timeout(ctx, 69, 7, 7, 1, 50);
> +	if (ret)
> +		v4l2_err(ctx->client, "init pm failed\n");
> +	return ret;
> +}
> +
> +static int max2175_recalibrate_adc(struct max2175 *ctx)
> +{
> +	int ret;
> +
> +	/* ADC Re-calibration */
> +	max2175_write(ctx, 150, 0xff);
> +	max2175_write(ctx, 205, 0xff);
> +	max2175_write(ctx, 147, 0x20);
> +	max2175_write(ctx, 147, 0x00);
> +	max2175_write(ctx, 202, 0x20);
> +	max2175_write(ctx, 202, 0x00);
> +
> +	ret = max2175_poll_timeout(ctx, 69, 4, 3, 3, 50);
> +	if (ret)
> +		v4l2_err(ctx->client, "adc recalibration failed\n");
> +	return ret;
> +}
> +
> +static u8 max2175_read_rom(struct max2175 *ctx, u8 row)
> +{
> +	u8 data;
> +
> +	max2175_write_bit(ctx, 56, 4, 0);
> +	max2175_write_bits(ctx, 56, 3, 0, row);
> +
> +	usleep_range(2000, 2500);
> +	max2175_read(ctx, 58, &data);
> +
> +	max2175_write_bits(ctx, 56, 3, 0, 0);
> +
> +	mxm_dbg(ctx, "read_rom: row %d data 0x%02x\n", row, data);
> +	return data;
> +}
> +
> +static void max2175_load_from_rom(struct max2175 *ctx)
> +{
> +	u8 data = 0;
> +
> +	data = max2175_read_rom(ctx, 0);
> +	ctx->rom_bbf_bw_am = data & 0x0f;
> +	max2175_write_bits(ctx, 81, 3, 0, data >> 4);
> +
> +	data = max2175_read_rom(ctx, 1);
> +	ctx->rom_bbf_bw_fm = data & 0x0f;
> +	ctx->rom_bbf_bw_dab = data >> 4;
> +
> +	data = max2175_read_rom(ctx, 2);
> +	max2175_write_bits(ctx, 82, 4, 0, data & 0x1f);
> +	max2175_write_bits(ctx, 82, 7, 5, data >> 5);
> +
> +	data = max2175_read_rom(ctx, 3);
> +	if (ctx->am_hiz) {
> +		data &= 0x0f;
> +		data |= max2175_read_rom(ctx, 7) & 0x40 >> 2;
> +		if (!data)
> +			data |= 2;
> +	} else {
> +		data = data & 0xf0 >> 4;
> +		data |= max2175_read_rom(ctx, 7) & 0x80 >> 3;
> +		if (!data)
> +			data |= 30;
> +	}
> +	max2175_write_bits(ctx, 80, 5, 0, data + 31);
> +
> +	data = max2175_read_rom(ctx, 6);
> +	max2175_write_bits(ctx, 81, 7, 6, data >> 6);
> +}
> +
> +static void max2175_load_full_fm_eu_1p0(struct max2175 *ctx)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(full_fm_eu_1p0); i++)
> +		max2175_write(ctx, i + 1, full_fm_eu_1p0[i]);
> +
> +	usleep_range(5000, 5500);
> +	ctx->decim_ratio = 36;
> +}
> +
> +static void max2175_load_full_fm_na_1p0(struct max2175 *ctx)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(full_fm_na_1p0); i++)
> +		max2175_write(ctx, i + 1, full_fm_na_1p0[i]);
> +
> +	usleep_range(5000, 5500);
> +	ctx->decim_ratio = 27;
> +}
> +
> +static int max2175_core_init(struct max2175 *ctx, u32 refout_bits)
> +{
> +	int ret;
> +
> +	/* MAX2175 uses 36.864MHz clock for EU & 40.154MHz for NA region */
> +	if (ctx->xtal_freq == MAX2175_EU_XTAL_FREQ)
> +		max2175_load_full_fm_eu_1p0(ctx);
> +	else
> +		max2175_load_full_fm_na_1p0(ctx);
> +
> +	/* The default settings assume master */
> +	if (!ctx->master)
> +		max2175_write_bit(ctx, 30, 7, 1);
> +
> +	mxm_dbg(ctx, "refout_bits %u\n", refout_bits);
> +
> +	/* Set REFOUT */
> +	max2175_write_bits(ctx, 56, 7, 5, refout_bits);
> +
> +	/* ADC Reset */
> +	max2175_write_bit(ctx, 99, 1, 0);
> +	usleep_range(1000, 1500);
> +	max2175_write_bit(ctx, 99, 1, 1);
> +
> +	/* Load ADC preset values */
> +	max2175_load_adc_presets(ctx);
> +
> +	/* Initialize the power management state machine */
> +	ret = max2175_init_power_manager(ctx);
> +	if (ret)
> +		return ret;
> +
> +	/* Recalibrate ADC */
> +	ret = max2175_recalibrate_adc(ctx);
> +	if (ret)
> +		return ret;
> +
> +	/* Load ROM values to appropriate registers */
> +	max2175_load_from_rom(ctx);
> +
> +	if (ctx->xtal_freq == MAX2175_EU_XTAL_FREQ) {
> +		/* Load FIR coefficients into bank 0 */
> +		max2175_set_filter_coeffs(ctx, MAX2175_CH_MSEL, 0,
> +					  ch_coeff_fmeu);
> +		max2175_set_filter_coeffs(ctx, MAX2175_EQ_MSEL, 0,
> +					  eq_coeff_fmeu1_ra02_m6db);
> +	} else {
> +		/* Load FIR coefficients into bank 0 */
> +		max2175_set_filter_coeffs(ctx, MAX2175_CH_MSEL, 0,
> +					  ch_coeff_fmna);
> +		max2175_set_filter_coeffs(ctx, MAX2175_EQ_MSEL, 0,
> +					  eq_coeff_fmna1_ra02_m6db);
> +	}
> +	mxm_dbg(ctx, "core initialized\n");
> +	return 0;
> +}
> +
> +static void max2175_s_ctrl_i2s_mode(struct max2175 *ctx, u32 i2s_mode)
> +{
> +	mxm_dbg(ctx, "s_ctrl_i2s_mode: %u resolved %d\n", i2s_mode,
> +		ctx->mode_resolved);
> +
> +	/*
> +	 * Update i2s mode on device only when mode is resolved & it is valid
> +	 * for the configured mode
> +	 */
> +	if (ctx->mode_resolved &&
> +	    max2175_i2s_rx_mode_valid(ctx, ctx->rx_mode->val, i2s_mode))
> +		max2175_update_i2s_mode(ctx, ctx->rx_mode->val, i2s_mode);
> +}
> +
> +static void max2175_s_ctrl_rx_mode(struct max2175 *ctx, u32 rx_mode)
> +{
> +	/* Load mode. Range check already done */
> +	max2175_set_rx_mode(ctx, rx_mode, ctx->hsls->val);
> +
> +	/* Get current i2s_mode and update if needed for given rx_mode */
> +	if (max2175_i2s_rx_mode_valid(ctx, rx_mode, ctx->i2s_mode->val))
> +		max2175_update_i2s_mode(ctx, rx_mode, ctx->i2s_mode->val);
> +	else
> +		ctx->i2s_mode->val = max2175_read_bits(ctx, 29, 2, 0);
> +
> +	mxm_dbg(ctx, "s_ctrl_rx_mode: %u curr freq %u\n", rx_mode, ctx->freq);
> +
> +	/* Check if current freq valid for mode & update */
> +	if (max2175_freq_rx_mode_valid(ctx, rx_mode, ctx->freq))
> +		max2175_tune_rf_freq(ctx, ctx->freq, ctx->hsls->val);
> +	else
> +		/* Use default freq of mode if current freq is not valid */
> +		max2175_tune_rf_freq(ctx, ctx->rx_modes[rx_mode].freq,
> +				     ctx->hsls->val);
> +}
> +
> +static int max2175_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct max2175 *ctx = max2175_from_ctrl_hdl(ctrl->handler);
> +	int ret = 0;
> +
> +	mxm_dbg(ctx, "s_ctrl: id 0x%x, val %u\n", ctrl->id, ctrl->val);
> +	switch (ctrl->id) {
> +	case V4L2_CID_MAX2175_I2S_ENABLE:
> +		max2175_i2s_enable(ctx, ctrl->val == 1);
> +		break;
> +	case V4L2_CID_MAX2175_I2S_MODE:
> +		max2175_s_ctrl_i2s_mode(ctx, ctrl->val);
> +		break;
> +	case V4L2_CID_MAX2175_HSLS:
> +		max2175_set_hsls(ctx, ctx->hsls->val);
> +		break;
> +	case V4L2_CID_MAX2175_RX_MODE:
> +		max2175_s_ctrl_rx_mode(ctx, ctrl->val);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int max2175_get_lna_gain(struct max2175 *ctx)
> +{
> +	int gain = 0;
> +	enum max2175_band band = max2175_read_bits(ctx, 5, 1, 0);
> +
> +	switch (band) {
> +	case MAX2175_BAND_AM:
> +		gain = max2175_read_bits(ctx, 51, 3, 1);
> +		break;
> +	case MAX2175_BAND_FM:
> +		gain = max2175_read_bits(ctx, 50, 3, 1);
> +		break;
> +	case MAX2175_BAND_VHF:
> +		gain = max2175_read_bits(ctx, 52, 3, 0);
> +		break;
> +	default:
> +		break;
> +	}
> +	return gain;
> +}
> +
> +static int max2175_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct max2175 *ctx = max2175_from_ctrl_hdl(ctrl->handler);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_RF_TUNER_LNA_GAIN:
> +		ctrl->val = max2175_get_lna_gain(ctx);
> +		break;
> +	case V4L2_CID_RF_TUNER_IF_GAIN:
> +		ctrl->val = max2175_read_bits(ctx, 49, 4, 0);
> +		break;
> +	case V4L2_CID_RF_TUNER_PLL_LOCK:
> +		ctrl->val = (max2175_read_bits(ctx, 60, 7, 6) == 3);
> +		break;
> +	}
> +	mxm_dbg(ctx, "g_volatile_ctrl: id 0x%x val %d\n", ctrl->id, ctrl->val);
> +	return 0;
> +};
> +
> +static int max2175_set_freq_and_mode(struct max2175 *ctx, u32 freq)
> +{
> +	u32 rx_mode;
> +	int ret;
> +
> +	/* Get band from frequency */
> +	ret = max2175_rx_mode_from_freq(ctx, freq, &rx_mode);
> +	if (ret)
> +		return ret;
> +
> +	mxm_dbg(ctx, "set_freq_and_mode: freq %u rx_mode %d\n", freq, rx_mode);
> +
> +	/* Load mode */
> +	max2175_set_rx_mode(ctx, rx_mode, ctx->hsls->val);
> +	ctx->rx_mode->val = rx_mode;
> +
> +	/* Get current i2s_mode and update if needed for given rx_mode */
> +	if (max2175_i2s_rx_mode_valid(ctx, rx_mode, ctx->i2s_mode->val))
> +		max2175_update_i2s_mode(ctx, rx_mode, ctx->i2s_mode->val);
> +	else
> +		ctx->i2s_mode->val = max2175_read_bits(ctx, 29, 2, 0);
> +
> +	/* Tune to the new freq given */
> +	return max2175_tune_rf_freq(ctx, freq, ctx->hsls->val);
> +}
> +
> +static int max2175_s_frequency(struct v4l2_subdev *sd,
> +			       const struct v4l2_frequency *vf)
> +{
> +	struct max2175 *ctx = max2175_from_sd(sd);
> +	u32 freq;
> +	int ret = 0;
> +
> +	mxm_dbg(ctx, "s_freq: new %u curr %u, mode_resolved %d\n",
> +		vf->frequency, ctx->freq, ctx->mode_resolved);
> +
> +	if (vf->tuner != 0)
> +		return -EINVAL;
> +
> +	freq = clamp(vf->frequency, ctx->bands_rf->rangelow,
> +		     ctx->bands_rf->rangehigh);
> +
> +	/* Check new freq valid for rx_mode if already resolved */
> +	if (ctx->mode_resolved &&
> +	    max2175_freq_rx_mode_valid(ctx, ctx->rx_mode->val, freq))
> +		ret = max2175_tune_rf_freq(ctx, freq, ctx->hsls->val);
> +	else
> +		/* Find default rx_mode for freq and tune to it */
> +		ret = max2175_set_freq_and_mode(ctx, freq);
> +
> +	mxm_dbg(ctx, "s_freq: ret %d curr %u mode_resolved %d mode %u\n",
> +		ret, ctx->freq, ctx->mode_resolved, ctx->rx_mode->val);
> +	return ret;
> +}
> +
> +static int max2175_g_frequency(struct v4l2_subdev *sd,
> +			       struct v4l2_frequency *vf)
> +{
> +	struct max2175 *ctx = max2175_from_sd(sd);
> +	int ret = 0;
> +
> +	if (vf->tuner != 0)
> +		return -EINVAL;
> +
> +	/* RF freq */
> +	vf->type = V4L2_TUNER_RF;
> +	vf->frequency = ctx->freq;
> +	return ret;
> +}
> +
> +static int max2175_enum_freq_bands(struct v4l2_subdev *sd,
> +			    struct v4l2_frequency_band *band)
> +{
> +	struct max2175 *ctx = max2175_from_sd(sd);
> +
> +	if (band->tuner == 0 && band->index == 0)
> +		*band = *ctx->bands_rf;
> +	else
> +		return -EINVAL;

This is a bit ugly. I would invert the condition and return -EINVAL.
Then assign *band and return 0.

> +
> +	return 0;
> +}
> +
> +static int max2175_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
> +{
> +	struct max2175 *ctx = max2175_from_sd(sd);
> +
> +	if (vt->index > 0)
> +		return -EINVAL;
> +
> +	strlcpy(vt->name, "RF", sizeof(vt->name));
> +	vt->type = V4L2_TUNER_RF;
> +	vt->capability = V4L2_TUNER_CAP_1HZ | V4L2_TUNER_CAP_FREQ_BANDS;
> +	vt->rangelow = ctx->bands_rf->rangelow;
> +	vt->rangehigh = ctx->bands_rf->rangehigh;
> +	return 0;
> +}
> +
> +static int max2175_s_tuner(struct v4l2_subdev *sd, const struct v4l2_tuner *vt)
> +{
> +	/* Check tuner index is valid */
> +	if (vt->index > 0)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_subdev_tuner_ops max2175_tuner_ops = {
> +	.s_frequency = max2175_s_frequency,
> +	.g_frequency = max2175_g_frequency,
> +	.enum_freq_bands = max2175_enum_freq_bands,
> +	.g_tuner = max2175_g_tuner,
> +	.s_tuner = max2175_s_tuner,
> +};
> +
> +static const struct v4l2_subdev_ops max2175_ops = {
> +	.tuner = &max2175_tuner_ops,
> +};
> +
> +static const struct v4l2_ctrl_ops max2175_ctrl_ops = {
> +	.s_ctrl = max2175_s_ctrl,
> +	.g_volatile_ctrl = max2175_g_volatile_ctrl,
> +};
> +
> +static const struct v4l2_ctrl_config max2175_i2s_en = {
> +	.ops = &max2175_ctrl_ops,
> +	.id = V4L2_CID_MAX2175_I2S_ENABLE,
> +	.name = "I2S Enable",
> +	.type = V4L2_CTRL_TYPE_BOOLEAN,
> +	.min = 0,
> +	.max = 1,
> +	.step = 1,
> +	.def = 1,
> +};
> +
> +static const char * const max2175_ctrl_i2s_modes[] = {
> +	[MAX2175_I2S_MODE0]	= "i2s mode 0",
> +	[MAX2175_I2S_MODE1]	= "i2s mode 1 (skipped)",
> +	[MAX2175_I2S_MODE2]	= "i2s mode 2",
> +	[MAX2175_I2S_MODE3]	= "i2s mode 3",
> +	[MAX2175_I2S_MODE4]	= "i2s mode 4",
> +};
> +
> +static const struct v4l2_ctrl_config max2175_i2s_mode = {
> +	.ops = &max2175_ctrl_ops,
> +	.id = V4L2_CID_MAX2175_I2S_MODE,
> +	.name = "I2S MODE value",
> +	.type = V4L2_CTRL_TYPE_MENU,
> +	.max = ARRAY_SIZE(max2175_ctrl_i2s_modes) - 1,
> +	.def = 0,
> +	.menu_skip_mask = 0x02,
> +	.qmenu = max2175_ctrl_i2s_modes,
> +};

Is this something that is changed dynamically? It looks more like a
device tree thing (it's not clear what it does, so obviously I
can't be sure).

> +
> +static const struct v4l2_ctrl_config max2175_hsls = {
> +	.ops = &max2175_ctrl_ops,
> +	.id = V4L2_CID_MAX2175_HSLS,
> +	.name = "HSLS above/below desired",
> +	.type = V4L2_CTRL_TYPE_INTEGER,
> +	.min = 0,
> +	.max = 1,
> +	.step = 1,
> +	.def = 1,
> +};
> +
> +static const char * const max2175_ctrl_eu_rx_modes[] = {
> +	[MAX2175_EU_FM_1_2]	= "EU FM 1.2",
> +	[MAX2175_DAB_1_2]	= "DAB 1.2",
> +};
> +
> +static const char * const max2175_ctrl_na_rx_modes[] = {
> +	[MAX2175_NA_FM_1_0]	= "NA FM 1.0",
> +	[MAX2175_NA_FM_2_0]	= "NA FM 2.0",
> +};
> +
> +static const struct v4l2_ctrl_config max2175_eu_rx_mode = {
> +	.ops = &max2175_ctrl_ops,
> +	.id = V4L2_CID_MAX2175_RX_MODE,
> +	.name = "RX MODE",
> +	.type = V4L2_CTRL_TYPE_MENU,
> +	.max = ARRAY_SIZE(max2175_ctrl_eu_rx_modes) - 1,
> +	.def = 0,
> +	.qmenu = max2175_ctrl_eu_rx_modes,
> +};
> +
> +static const struct v4l2_ctrl_config max2175_na_rx_mode = {
> +	.ops = &max2175_ctrl_ops,
> +	.id = V4L2_CID_MAX2175_RX_MODE,
> +	.name = "RX MODE",
> +	.type = V4L2_CTRL_TYPE_MENU,
> +	.max = ARRAY_SIZE(max2175_ctrl_na_rx_modes) - 1,
> +	.def = 0,
> +	.qmenu = max2175_ctrl_na_rx_modes,
> +};

Please document all these controls better. This is part of the public API, so
you need to give more information what this means exactly.

> +
> +static int max2175_refout_load_to_bits(struct i2c_client *client, u32 load,
> +				       u32 *bits)
> +{
> +	if (load >= 0 && load <= 40)
> +		*bits = load / 10;
> +	else if (load >= 60 && load <= 70)
> +		*bits = load / 10 - 1;
> +	else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int max2175_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct max2175 *ctx;
> +	struct v4l2_subdev *sd;
> +	struct v4l2_ctrl_handler *hdl;
> +	struct clk *clk;
> +	struct regmap *regmap;
> +	bool master = true, am_hiz = false;
> +	u32 refout_load, refout_bits = 0;	/* REFOUT disabled */
> +	int ret;
> +
> +	/* Parse DT properties */
> +	if (of_find_property(client->dev.of_node, "maxim,slave", NULL))
> +		master = false;
> +
> +	if (of_find_property(client->dev.of_node, "maxim,am-hiz", NULL))
> +		am_hiz = true;
> +
> +	if (!of_property_read_u32(client->dev.of_node, "maxim,refout-load-pF",
> +				  &refout_load)) {
> +		ret = max2175_refout_load_to_bits(client, refout_load,
> +						  &refout_bits);
> +		if (ret) {
> +			dev_err(&client->dev, "invalid refout_load %u\n",
> +				refout_load);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	clk = devm_clk_get(&client->dev, "xtal");
> +	if (IS_ERR(clk)) {
> +		ret = PTR_ERR(clk);
> +		dev_err(&client->dev, "cannot get xtal clock %d\n", ret);
> +		return -ENODEV;
> +	}
> +
> +	regmap = devm_regmap_init_i2c(client, &max2175_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		ret = PTR_ERR(regmap);
> +		dev_err(&client->dev, "regmap init failed %d\n", ret);
> +		return -ENODEV;
> +	}
> +
> +	/* Alloc tuner context */
> +	ctx = devm_kzalloc(&client->dev, sizeof(*ctx), GFP_KERNEL);
> +	if (ctx == NULL)
> +		return -ENOMEM;
> +
> +	sd = &ctx->sd;
> +	ctx->master = master;
> +	ctx->am_hiz = am_hiz;
> +	ctx->mode_resolved = false;
> +	ctx->regmap = regmap;
> +	ctx->xtal_freq = clk_get_rate(clk);
> +	dev_info(&client->dev, "xtal freq %luHz\n", ctx->xtal_freq);
> +
> +	v4l2_i2c_subdev_init(sd, client, &max2175_ops);
> +	ctx->client = client;
> +
> +	sd->flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> +	/* Controls */
> +	hdl = &ctx->ctrl_hdl;
> +	ret = v4l2_ctrl_handler_init(hdl, 7);
> +	if (ret) {
> +		dev_err(&client->dev, "ctrl handler init failed\n");
> +		goto err;
> +	}
> +
> +	ctx->lna_gain = v4l2_ctrl_new_std(hdl, &max2175_ctrl_ops,
> +					  V4L2_CID_RF_TUNER_LNA_GAIN,
> +					  0, 15, 1, 2);
> +	ctx->lna_gain->flags |= (V4L2_CTRL_FLAG_VOLATILE |
> +				 V4L2_CTRL_FLAG_READ_ONLY);
> +	ctx->if_gain = v4l2_ctrl_new_std(hdl, &max2175_ctrl_ops,
> +					 V4L2_CID_RF_TUNER_IF_GAIN,
> +					 0, 31, 1, 0);
> +	ctx->if_gain->flags |= (V4L2_CTRL_FLAG_VOLATILE |
> +				V4L2_CTRL_FLAG_READ_ONLY);
> +	ctx->pll_lock = v4l2_ctrl_new_std(hdl, &max2175_ctrl_ops,
> +					  V4L2_CID_RF_TUNER_PLL_LOCK,
> +					  0, 1, 1, 0);
> +	ctx->pll_lock->flags |= (V4L2_CTRL_FLAG_VOLATILE |
> +				 V4L2_CTRL_FLAG_READ_ONLY);
> +	ctx->i2s_en = v4l2_ctrl_new_custom(hdl, &max2175_i2s_en, NULL);
> +	ctx->i2s_mode = v4l2_ctrl_new_custom(hdl, &max2175_i2s_mode, NULL);
> +	ctx->hsls = v4l2_ctrl_new_custom(hdl, &max2175_hsls, NULL);
> +
> +	if (ctx->xtal_freq == MAX2175_EU_XTAL_FREQ) {
> +		ctx->rx_mode = v4l2_ctrl_new_custom(hdl,
> +						    &max2175_eu_rx_mode, NULL);
> +		ctx->rx_modes = eu_rx_modes;
> +		ctx->bands_rf = &eu_bands_rf;
> +	} else {
> +		ctx->rx_mode = v4l2_ctrl_new_custom(hdl,
> +						    &max2175_na_rx_mode, NULL);
> +		ctx->rx_modes = na_rx_modes;
> +		ctx->bands_rf = &na_bands_rf;
> +	}
> +	ctx->sd.ctrl_handler = &ctx->ctrl_hdl;
> +
> +	/* Set the defaults */
> +	ctx->freq = ctx->bands_rf->rangelow;
> +
> +	/* Register subdev */
> +	ret = v4l2_async_register_subdev(sd);
> +	if (ret) {
> +		dev_err(&client->dev, "register subdev failed\n");
> +		goto err_reg;
> +	}
> +
> +	/* Initialize device */
> +	ret = max2175_core_init(ctx, refout_bits);
> +	if (ret)
> +		goto err_init;
> +
> +	dev_info(&client->dev, "probed\n");
> +	return 0;
> +
> +err_init:
> +	v4l2_async_unregister_subdev(sd);
> +err_reg:
> +	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> +err:
> +	return ret;
> +}
> +
> +static int max2175_remove(struct i2c_client *client)
> +{
> +	struct v4l2_subdev *sd = i2c_get_clientdata(client);
> +	struct max2175 *ctx = max2175_from_sd(sd);
> +
> +	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
> +	v4l2_async_unregister_subdev(sd);
> +	dev_info(&client->dev, "removed\n");
> +	return 0;
> +}
> +
> +static const struct i2c_device_id max2175_id[] = {
> +	{ DRIVER_NAME, 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, max2175_id);
> +
> +static const struct of_device_id max2175_of_ids[] = {
> +	{ .compatible = "maxim, max2175", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, max2175_of_ids);
> +
> +static struct i2c_driver max2175_driver = {
> +	.driver = {
> +		.name	= DRIVER_NAME,
> +		.of_match_table = max2175_of_ids,
> +	},
> +	.probe		= max2175_probe,
> +	.remove		= max2175_remove,
> +	.id_table	= max2175_id,
> +};
> +
> +module_i2c_driver(max2175_driver);
> +
> +MODULE_DESCRIPTION("Maxim MAX2175 RF to Bits tuner driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>");
> diff --git a/drivers/media/i2c/max2175/max2175.h b/drivers/media/i2c/max2175/max2175.h
> new file mode 100644
> index 0000000..2d858aa
> --- /dev/null
> +++ b/drivers/media/i2c/max2175/max2175.h
> @@ -0,0 +1,108 @@
> +/*
> + * Maxim Integrated MAX2175 RF to Bits tuner driver
> + *
> + * This driver & most of the hard coded values are based on the reference
> + * application delivered by Maxim for this chip.
> + *
> + * Copyright (C) 2016 Maxim Integrated Products
> + * Copyright (C) 2016 Renesas Electronics Corporation
> + *
> + * 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 __MAX2175_H__
> +#define __MAX2175_H__
> +
> +#define MAX2175_EU_XTAL_FREQ	36864000	/* In Hz */
> +#define MAX2175_NA_XTAL_FREQ	40186125	/* In Hz */
> +
> +enum max2175_region {
> +	MAX2175_REGION_EU = 0,	/* Europe */
> +	MAX2175_REGION_NA,	/* North America */
> +};
> +
> +
> +enum max2175_band {
> +	MAX2175_BAND_AM = 0,
> +	MAX2175_BAND_FM,
> +	MAX2175_BAND_VHF,
> +	MAX2175_BAND_L,
> +};
> +
> +enum max2175_eu_mode {
> +	/* EU modes */
> +	MAX2175_EU_FM_1_2 = 0,
> +	MAX2175_DAB_1_2,
> +
> +	/* Other possible modes to add in future
> +	 * MAX2175_DAB_1_0,
> +	 * MAX2175_DAB_1_3,
> +	 * MAX2175_EU_FM_2_2,
> +	 * MAX2175_EU_FMHD_4_0,
> +	 * MAX2175_EU_AM_1_0,
> +	 * MAX2175_EU_AM_2_2,
> +	 */
> +};
> +
> +enum max2175_na_mode {
> +	/* NA modes */
> +	MAX2175_NA_FM_1_0 = 0,
> +	MAX2175_NA_FM_2_0,
> +
> +	/* Other possible modes to add in future
> +	 * MAX2175_NA_FMHD_1_0,
> +	 * MAX2175_NA_FMHD_1_2,
> +	 * MAX2175_NA_AM_1_0,
> +	 * MAX2175_NA_AM_1_2,
> +	 */
> +};
> +
> +/* Supported I2S modes */
> +enum {
> +	MAX2175_I2S_MODE0 = 0,
> +	MAX2175_I2S_MODE1,
> +	MAX2175_I2S_MODE2,
> +	MAX2175_I2S_MODE3,
> +	MAX2175_I2S_MODE4,
> +};
> +
> +/* Coefficient table groups */
> +enum {
> +	MAX2175_CH_MSEL = 0,
> +	MAX2175_EQ_MSEL,
> +	MAX2175_AA_MSEL,
> +};
> +
> +/* HSLS LO injection polarity */
> +enum {
> +	MAX2175_LO_BELOW_DESIRED = 0,
> +	MAX2175_LO_ABOVE_DESIRED,
> +};
> +
> +/* Channel FSM modes */
> +enum max2175_csm_mode {
> +	MAX2175_LOAD_TO_BUFFER = 0,
> +	MAX2175_PRESET_TUNE,
> +	MAX2175_SEARCH,
> +	MAX2175_AF_UPDATE,
> +	MAX2175_JUMP_FAST_TUNE,
> +	MAX2175_CHECK,
> +	MAX2175_LOAD_AND_SWAP,
> +	MAX2175_END,
> +	MAX2175_BUFFER_PLUS_PRESET_TUNE,
> +	MAX2175_BUFFER_PLUS_SEARCH,
> +	MAX2175_BUFFER_PLUS_AF_UPDATE,
> +	MAX2175_BUFFER_PLUS_JUMP_FAST_TUNE,
> +	MAX2175_BUFFER_PLUS_CHECK,
> +	MAX2175_BUFFER_PLUS_LOAD_AND_SWAP,
> +	MAX2175_NO_ACTION
> +};
> +
> +#endif /* __MAX2175_H__ */
> 

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH 3/5] media: Add new SDR formats SC16, SC18 & SC20
From: Hans Verkuil @ 2016-11-11 13:24 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram, robh+dt, mark.rutland, mchehab,
	sakari.ailus, crope
  Cc: chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <1478706284-59134-4-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
> This patch adds support for the three new SDR formats. These formats
> were prefixed with "sliced" indicating I data constitutes the top half and
> Q data constitutes the bottom half of the received buffer.

The standard terminology for video formats is "planar". I am leaning towards
using that here as well.

Any opinions on this?

	Hans

> 
> V4L2_SDR_FMT_SCU16BE - 14-bit complex (I & Q) unsigned big-endian sample
> inside 16-bit. V4L2 FourCC: SC16
> 
> V4L2_SDR_FMT_SCU18BE - 16-bit complex (I & Q) unsigned big-endian sample
> inside 18-bit. V4L2 FourCC: SC18
> 
> V4L2_SDR_FMT_SCU20BE - 18-bit complex (I & Q) unsigned big-endian sample
> inside 20-bit. V4L2 FourCC: SC20
> 
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
>  include/uapi/linux/videodev2.h       | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 181381d..d36b386 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1207,6 +1207,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  	case V4L2_SDR_FMT_CS8:		descr = "Complex S8"; break;
>  	case V4L2_SDR_FMT_CS14LE:	descr = "Complex S14LE"; break;
>  	case V4L2_SDR_FMT_RU12LE:	descr = "Real U12LE"; break;
> +	case V4L2_SDR_FMT_SCU16BE:	descr = "Sliced Complex U16BE"; break;
> +	case V4L2_SDR_FMT_SCU18BE:	descr = "Sliced Complex U18BE"; break;
> +	case V4L2_SDR_FMT_SCU20BE:	descr = "Sliced Complex U20BE"; break;
>  	case V4L2_TCH_FMT_DELTA_TD16:	descr = "16-bit signed deltas"; break;
>  	case V4L2_TCH_FMT_DELTA_TD08:	descr = "8-bit signed deltas"; break;
>  	case V4L2_TCH_FMT_TU16:		descr = "16-bit unsigned touch data"; break;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 4364ce6..34a9c30 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -666,6 +666,9 @@ struct v4l2_pix_format {
>  #define V4L2_SDR_FMT_CS8          v4l2_fourcc('C', 'S', '0', '8') /* complex s8 */
>  #define V4L2_SDR_FMT_CS14LE       v4l2_fourcc('C', 'S', '1', '4') /* complex s14le */
>  #define V4L2_SDR_FMT_RU12LE       v4l2_fourcc('R', 'U', '1', '2') /* real u12le */
> +#define V4L2_SDR_FMT_SCU16BE	  v4l2_fourcc('S', 'C', '1', '6') /* sliced complex u16be */
> +#define V4L2_SDR_FMT_SCU18BE	  v4l2_fourcc('S', 'C', '1', '8') /* sliced complex u18be */
> +#define V4L2_SDR_FMT_SCU20BE	  v4l2_fourcc('S', 'C', '2', '0') /* sliced complex u20be */
>  
>  /* Touch formats - used for Touch devices */
>  #define V4L2_TCH_FMT_DELTA_TD16	v4l2_fourcc('T', 'D', '1', '6') /* 16-bit signed deltas */
> 

^ permalink raw reply

* Re: [PATCH v5 6/8] Documentation: bindings: add compatible specific to legacy SCPI protocol
From: Rob Herring @ 2016-11-11 13:34 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Olof Johansson, Neil Armstrong,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <ed58424e-e538-227a-37b5-b35fbe4f96ba-5wv7dgnIgG8@public.gmane.org>

On Fri, Nov 11, 2016 at 1:48 AM, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org> wrote:
> On 10/11/16 19:03, Olof Johansson wrote:
>> On Thu, Nov 10, 2016 at 6:34 AM, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>> wrote:

[...]

>>> E.g. Amlogic follows most of the legacy protocol though it deviates in
>>> couple of things which we can handle with platform specific compatible
>>> (in the following patch in the series). When another user(Rockchip ?)
>>> make use of this legacy protocol, we can start using those platform
>>> specific compatible for deviations only.
>>>
>>> Is that not acceptable ?
>>
>>
>> If there's no shared legacy feature set, then it's probably less
>> useful to have a shared less precise compatible value.
>>
>
> There is and will be some shared feature set for sure. At the least the
> standard command set will be shared.
>
>> What the main point I was trying to get across was that we shouldn't
>> expand the generic binding with per-vendor compatible fields, instead
>> we should have those as extensions on the side.
>>
>
> Yes I get the point. We will have per-vendor compatibles for handle the
> deviations but generic one to handle the shared set.
>
>> I'm also a little apprehensive of using "legacy", it goes in the same
>> bucket as "misc". At some point 1.0 will be legacy too, etc.
>>
>
> True and I agree, how about "arm,scpi-pre-1.0" instead ?

That's still meaningless. Convince me that multiple implementations
are identical, then we can have a common property. For example, how
many releases did ARM make before 1.0.

Rob
--
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 5/5] media: platform: rcar_drif: Add DRIF support
From: Hans Verkuil @ 2016-11-11 13:38 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram, robh+dt, mark.rutland, mchehab,
	sakari.ailus, crope
  Cc: chris.paterson2, laurent.pinchart, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <1478706284-59134-6-git-send-email-ramesh.shanmugasundaram@bp.renesas.com>

On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
> This patch adds Digital Radio Interface (DRIF) support to R-Car Gen3 SoCs.
> The driver exposes each instance of DRIF as a V4L2 SDR device. A DRIF
> device represents a channel and each channel can have one or two
> sub-channels respectively depending on the target board.
> 
> DRIF supports only Rx functionality. It receives samples from a RF
> frontend tuner chip it is interfaced with. The combination of DRIF and the
> tuner device, which is registered as a sub-device, determines the receive
> sample rate and format.
> 
> In order to be compliant as a V4L2 SDR device, DRIF needs to bind with
> the tuner device, which can be provided by a third party vendor. DRIF acts
> as a slave device and the tuner device acts as a master transmitting the
> samples. The driver allows asynchronous binding of a tuner device that
> is registered as a v4l2 sub-device. The driver can learn about the tuner
> it is interfaced with based on port endpoint properties of the device in
> device tree. The V4L2 SDR device inherits the controls exposed by the
> tuner device.
> 
> The device can also be configured to use either one or both of the data
> pins at runtime based on the master (tuner) configuration.
> 
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
> ---
>  .../devicetree/bindings/media/renesas,drif.txt     |  136 ++
>  drivers/media/platform/Kconfig                     |   25 +
>  drivers/media/platform/Makefile                    |    1 +
>  drivers/media/platform/rcar_drif.c                 | 1574 ++++++++++++++++++++
>  4 files changed, 1736 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,drif.txt
>  create mode 100644 drivers/media/platform/rcar_drif.c
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,drif.txt b/Documentation/devicetree/bindings/media/renesas,drif.txt
> new file mode 100644
> index 0000000..d65368a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,drif.txt
> @@ -0,0 +1,136 @@
> +Renesas R-Car Gen3 Digital Radio Interface controller (DRIF)
> +------------------------------------------------------------
> +
> +R-Car Gen3 DRIF is a serial slave device. It interfaces with a master
> +device as shown below
> +
> ++---------------------+                +---------------------+
> +|                     |-----SCK------->|CLK                  |
> +|       Master        |-----SS-------->|SYNC  DRIFn (slave)  |
> +|                     |-----SD0------->|D0                   |
> +|                     |-----SD1------->|D1                   |
> ++---------------------+                +---------------------+
> +
> +Each DRIF channel (drifn) consists of two sub-channels (drifn0 & drifn1).
> +The sub-channels are like two individual channels in itself that share the
> +common CLK & SYNC. Each sub-channel has it's own dedicated resources like
> +irq, dma channels, address space & clock.
> +
> +The device tree model represents the channel and each of it's sub-channel
> +as a separate node. The parent channel ties the sub-channels together with
> +their phandles.
> +
> +Required properties of a sub-channel:
> +-------------------------------------
> +- compatible: "renesas,r8a7795-drif" if DRIF controller is a part of R8A7795 SoC.
> +	      "renesas,rcar-gen3-drif" for a generic R-Car Gen3 compatible device.
> +	      When compatible with the generic version, nodes must list the
> +	      SoC-specific version corresponding to the platform first
> +	      followed by the generic version.
> +- reg: offset and length of that sub-channel.
> +- interrupts: associated with that sub-channel.
> +- clocks: phandle and clock specifier of that sub-channel.
> +- clock-names: clock input name string: "fck".
> +- dmas: phandles to the DMA channel of that sub-channel.
> +- dma-names: names of the DMA channel: "rx".
> +
> +Optional properties of a sub-channel:
> +-------------------------------------
> +- power-domains: phandle to the respective power domain.
> +
> +Required properties of a channel:
> +---------------------------------
> +- pinctrl-0: pin control group to be used for this channel.
> +- pinctrl-names: must be "default".
> +- sub-channels : phandles to the two sub-channels.
> +
> +Optional properties of a channel:
> +---------------------------------
> +- port: child port node of a channel that defines the local and remote
> +        endpoints. The remote endpoint is assumed to be a tuner subdevice
> +	endpoint.
> +- renesas,syncmd       : sync mode
> +			 0 (Frame start sync pulse mode. 1-bit width pulse
> +			    indicates start of a frame)
> +			 1 (L/R sync or I2S mode) (default)
> +- renesas,lsb-first    : empty property indicates lsb bit is received first.
> +			 When not defined msb bit is received first (default)
> +- renesas,syncac-pol-high  : empty property indicates sync signal polarity.
> +			 When defined, active high or high->low sync signal.
> +			 When not defined, active low or low->high sync signal
> +			 (default)
> +- renesas,dtdl         : delay between sync signal and start of reception.
> +			 Must contain one of the following values:
> +			 0   (no bit delay)
> +			 50  (0.5-clock-cycle delay)
> +			 100 (1-clock-cycle delay) (default)
> +			 150 (1.5-clock-cycle delay)
> +			 200 (2-clock-cycle delay)
> +- renesas,syncdl       : delay between end of reception and sync signal edge.
> +			 Must contain one of the following values:
> +			 0   (no bit delay) (default)
> +			 50  (0.5-clock-cycle delay)
> +			 100 (1-clock-cycle delay)
> +			 150 (1.5-clock-cycle delay)
> +			 200 (2-clock-cycle delay)
> +			 300 (3-clock-cycle delay)
> +
> +Example
> +--------
> +
> +SoC common dtsi file
> +
> +		drif00: rif@e6f40000 {
> +			compatible = "renesas,r8a7795-drif",
> +				     "renesas,rcar-gen3-drif";
> +			reg = <0 0xe6f40000 0 0x64>;
> +			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 515>;
> +			clock-names = "fck";
> +			dmas = <&dmac1 0x20>, <&dmac2 0x20>;
> +			dma-names = "rx", "rx";
> +			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> +			status = "disabled";
> +		};
> +
> +		drif01: rif@e6f50000 {
> +			compatible = "renesas,r8a7795-drif",
> +				     "renesas,rcar-gen3-drif";
> +			reg = <0 0xe6f50000 0 0x64>;
> +			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 514>;
> +			clock-names = "fck";
> +			dmas = <&dmac1 0x22>, <&dmac2 0x22>;
> +			dma-names = "rx", "rx";
> +			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
> +			status = "disabled";
> +		};
> +
> +		drif0: rif@0 {
> +			compatible = "renesas,r8a7795-drif",
> +				     "renesas,rcar-gen3-drif";
> +			sub-channels = <&drif00>, <&drif01>;
> +			status = "disabled";
> +		};
> +
> +Board specific dts file
> +
> +&drif00 {
> +	status = "okay";
> +};
> +
> +&drif01 {
> +	status = "okay";
> +};
> +
> +&drif0 {
> +	pinctrl-0 = <&drif0_pins>;
> +	pinctrl-names = "default";
> +	renesas,syncac-pol-high;
> +	status = "okay";
> +	port {
> +		drif0_ep: endpoint {
> +		     remote-endpoint = <&tuner_subdev_ep>;
> +		};
> +	};
> +};
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 754edbf1..0ae83a8 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -393,3 +393,28 @@ menuconfig DVB_PLATFORM_DRIVERS
>  if DVB_PLATFORM_DRIVERS
>  source "drivers/media/platform/sti/c8sectpfe/Kconfig"
>  endif #DVB_PLATFORM_DRIVERS
> +
> +menuconfig SDR_PLATFORM_DRIVERS
> +	bool "SDR platform devices"
> +	depends on MEDIA_SDR_SUPPORT
> +	default n
> +	---help---
> +	  Say Y here to enable support for platform-specific SDR Drivers.
> +
> +if SDR_PLATFORM_DRIVERS
> +
> +config VIDEO_RCAR_DRIF
> +	tristate "Renesas Digitial Radio Interface (DRIF)"
> +	depends on VIDEO_V4L2 && HAS_DMA
> +	depends on ARCH_RENESAS
> +	select VIDEOBUF2_VMALLOC
> +	---help---
> +	  Say Y if you want to enable R-Car Gen3 DRIF support. DRIF is Digital
> +	  Radio Interface that interfaces with an RF front end chip. It is a
> +	  receiver of digital data which uses DMA to transfer received data to
> +	  a configured location for an application to use.
> +
> +	  To compile this driver as a module, choose M here; the module
> +	  will be called rcar_drif.
> +
> +endif # SDR_PLATFORM_DRIVERS
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index f842933..49ce238 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_SOC_CAMERA)		+= soc_camera/
>  
>  obj-$(CONFIG_VIDEO_RENESAS_FCP) 	+= rcar-fcp.o
>  obj-$(CONFIG_VIDEO_RENESAS_JPU) 	+= rcar_jpu.o
> +obj-$(CONFIG_VIDEO_RCAR_DRIF)		+= rcar_drif.o
>  obj-$(CONFIG_VIDEO_RENESAS_VSP1)	+= vsp1/
>  
>  obj-y	+= omap/
> diff --git a/drivers/media/platform/rcar_drif.c b/drivers/media/platform/rcar_drif.c
> new file mode 100644
> index 0000000..34dc282
> --- /dev/null
> +++ b/drivers/media/platform/rcar_drif.c
> @@ -0,0 +1,1574 @@

<snip>

+#define for_each_rcar_drif_subdev(sd, tmp, ch)				\
+	list_for_each_entry_safe(sd, tmp, &ch->v4l2_dev.subdevs, list)
+

Please don't use this. media/v4l2-device.h has a bunch of similar functions
for this. Use those instead.

<snip>

> +static int rcar_drif_querycap(struct file *file, void *fh,
> +			      struct v4l2_capability *cap)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +
> +	strlcpy(cap->driver, KBUILD_MODNAME, sizeof(cap->driver));
> +	strlcpy(cap->card, ch->vdev.name, sizeof(cap->card));
> +	cap->device_caps = V4L2_CAP_SDR_CAPTURE | V4L2_CAP_TUNER |
> +				   V4L2_CAP_STREAMING | V4L2_CAP_READWRITE;
> +	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;

Set device_caps in struct video_device and drop it here.

The core will fill in cap->device_caps and cap->capabilities for you.

> +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> +		 ch->vdev.name);
> +	return 0;
> +}
> +
> +static int rcar_drif_set_default_format(struct rcar_drif_chan *ch)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < NUM_FORMATS; i++) {
> +		/* Find any matching fmt and set it as default */
> +		if (ch->num_hw_schans == formats[i].num_schans) {
> +			ch->fmt_idx = i;
> +			ch->cur_schans_mask = ch->hw_schans_mask;
> +			ch->num_cur_schans = ch->num_hw_schans;
> +			dev_dbg(ch->dev, "default fmt[%u]: mask %lu num %u\n",
> +				i, ch->cur_schans_mask, ch->num_cur_schans);
> +			return 0;
> +		}
> +	}
> +	dev_err(ch->dev, "no matching sdr fmt found\n");
> +	return -EINVAL;
> +}
> +
> +static int rcar_drif_enum_fmt_sdr_cap(struct file *file, void *priv,
> +				      struct v4l2_fmtdesc *f)
> +{
> +	if (f->index >= NUM_FORMATS)
> +		return -EINVAL;
> +
> +	strlcpy(f->description, formats[f->index].name, sizeof(f->description));

Drop this. The core fills that in for you.

> +	f->pixelformat = formats[f->index].pixelformat;
> +	return 0;
> +}
> +
> +static int rcar_drif_g_fmt_sdr_cap(struct file *file, void *priv,
> +				   struct v4l2_format *f)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +
> +	f->fmt.sdr.pixelformat = formats[ch->fmt_idx].pixelformat;
> +	f->fmt.sdr.buffersize = formats[ch->fmt_idx].buffersize;
> +	memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
> +	return 0;
> +}
> +
> +static int rcar_drif_s_fmt_sdr_cap(struct file *file, void *priv,
> +				   struct v4l2_format *f)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +	struct vb2_queue *q = &ch->vb_queue;
> +	unsigned int i;
> +
> +	if (vb2_is_busy(q))
> +		return -EBUSY;
> +
> +	memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
> +	for (i = 0; i < NUM_FORMATS; i++) {
> +		if (formats[i].pixelformat == f->fmt.sdr.pixelformat) {
> +			ch->fmt_idx  = i;
> +			f->fmt.sdr.buffersize = formats[i].buffersize;
> +
> +			/*
> +			 * If a format demands one sub-channel only out of two
> +			 * enabled sub-channels then pick the 0th sub-channel
> +			 */
> +			if (formats[i].num_schans < ch->num_hw_schans) {
> +				ch->cur_schans_mask = BIT(0);	/* Enable D0 */
> +				ch->num_cur_schans = formats[i].num_schans;
> +			} else {
> +				ch->cur_schans_mask = ch->hw_schans_mask;
> +				ch->num_cur_schans = ch->num_hw_schans;
> +			}
> +
> +			rdrif_dbg(1, ch, "cur: idx %u mask %lu num %u\n",
> +				  i, ch->cur_schans_mask, ch->num_cur_schans);
> +			return 0;
> +		}
> +	}
> +
> +	if (rcar_drif_set_default_format(ch))
> +		return -EINVAL;
> +
> +	f->fmt.sdr.pixelformat = formats[ch->fmt_idx].pixelformat;
> +	f->fmt.sdr.buffersize = formats[ch->fmt_idx].buffersize;
> +	return 0;
> +}
> +
> +static int rcar_drif_try_fmt_sdr_cap(struct file *file, void *priv,
> +				     struct v4l2_format *f)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +	unsigned int i;
> +
> +	memset(f->fmt.sdr.reserved, 0, sizeof(f->fmt.sdr.reserved));
> +	for (i = 0; i < NUM_FORMATS; i++) {
> +		if (formats[i].pixelformat == f->fmt.sdr.pixelformat) {
> +			f->fmt.sdr.buffersize = formats[i].buffersize;
> +			return 0;
> +		}
> +	}
> +
> +	f->fmt.sdr.pixelformat = formats[ch->fmt_idx].pixelformat;
> +	f->fmt.sdr.buffersize = formats[ch->fmt_idx].buffersize;
> +	return 0;
> +}
> +
> +/* Tuner subdev ioctls */
> +static int rcar_drif_enum_freq_bands(struct file *file, void *priv,
> +				     struct v4l2_frequency_band *band)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +	struct v4l2_subdev *sd, *tmp;
> +	int ret = 0;
> +
> +	for_each_rcar_drif_subdev(sd, tmp, ch) {
> +		ret = v4l2_subdev_call(sd, tuner, enum_freq_bands, band);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +static int rcar_drif_g_frequency(struct file *file, void *priv,
> +				 struct v4l2_frequency *f)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +	struct v4l2_subdev *sd, *tmp;
> +	int ret = 0;
> +
> +	for_each_rcar_drif_subdev(sd, tmp, ch) {
> +		ret = v4l2_subdev_call(sd, tuner, g_frequency, f);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +static int rcar_drif_s_frequency(struct file *file, void *priv,
> +				 const struct v4l2_frequency *f)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +	struct v4l2_subdev *sd, *tmp;
> +	int ret = 0;
> +
> +	for_each_rcar_drif_subdev(sd, tmp, ch) {
> +		ret = v4l2_subdev_call(sd, tuner, s_frequency, f);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +static int rcar_drif_g_tuner(struct file *file, void *priv,
> +			     struct v4l2_tuner *vt)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +	struct v4l2_subdev *sd, *tmp;
> +	int ret = 0;
> +
> +	for_each_rcar_drif_subdev(sd, tmp, ch) {
> +		ret = v4l2_subdev_call(sd, tuner, g_tuner, vt);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +static int rcar_drif_s_tuner(struct file *file, void *priv,
> +			     const struct v4l2_tuner *vt)
> +{
> +	struct rcar_drif_chan *ch = video_drvdata(file);
> +	struct v4l2_subdev *sd, *tmp;
> +	int ret = 0;
> +
> +	for_each_rcar_drif_subdev(sd, tmp, ch) {
> +		ret = v4l2_subdev_call(sd, tuner, s_tuner, vt);
> +		if (ret)
> +			break;
> +	}
> +	return ret;
> +}
> +
> +static const struct v4l2_ioctl_ops rcar_drif_ioctl_ops = {
> +	.vidioc_querycap          = rcar_drif_querycap,
> +
> +	.vidioc_enum_fmt_sdr_cap  = rcar_drif_enum_fmt_sdr_cap,
> +	.vidioc_g_fmt_sdr_cap     = rcar_drif_g_fmt_sdr_cap,
> +	.vidioc_s_fmt_sdr_cap     = rcar_drif_s_fmt_sdr_cap,
> +	.vidioc_try_fmt_sdr_cap   = rcar_drif_try_fmt_sdr_cap,
> +
> +	.vidioc_reqbufs           = vb2_ioctl_reqbufs,
> +	.vidioc_create_bufs       = vb2_ioctl_create_bufs,
> +	.vidioc_prepare_buf       = vb2_ioctl_prepare_buf,
> +	.vidioc_querybuf          = vb2_ioctl_querybuf,
> +	.vidioc_qbuf              = vb2_ioctl_qbuf,
> +	.vidioc_dqbuf             = vb2_ioctl_dqbuf,
> +
> +	.vidioc_streamon          = vb2_ioctl_streamon,
> +	.vidioc_streamoff         = vb2_ioctl_streamoff,
> +
> +	.vidioc_s_frequency       = rcar_drif_s_frequency,
> +	.vidioc_g_frequency       = rcar_drif_g_frequency,
> +	.vidioc_s_tuner		  = rcar_drif_s_tuner,
> +	.vidioc_g_tuner		  = rcar_drif_g_tuner,
> +	.vidioc_enum_freq_bands   = rcar_drif_enum_freq_bands,
> +	.vidioc_subscribe_event   = v4l2_ctrl_subscribe_event,
> +	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +	.vidioc_log_status        = v4l2_ctrl_log_status,
> +};
> +
> +static const struct v4l2_file_operations rcar_drif_fops = {
> +	.owner                    = THIS_MODULE,
> +	.open                     = v4l2_fh_open,
> +	.release                  = vb2_fop_release,
> +	.read                     = vb2_fop_read,
> +	.poll                     = vb2_fop_poll,
> +	.mmap                     = vb2_fop_mmap,
> +	.unlocked_ioctl           = video_ioctl2,
> +};
> +
> +static struct video_device rcar_drif_vdev = {
> +	.name                     = "R-Car DRIF",
> +	.release                  = video_device_release_empty,
> +	.fops                     = &rcar_drif_fops,
> +	.ioctl_ops                = &rcar_drif_ioctl_ops,
> +};
> +
> +static int rcar_drif_notify_bound(struct v4l2_async_notifier *notifier,
> +				   struct v4l2_subdev *subdev,
> +				   struct v4l2_async_subdev *asd)
> +{
> +	struct rcar_drif_chan *ch =
> +		container_of(notifier, struct rcar_drif_chan, notifier);
> +
> +	/* Nothing to do at this point */
> +	rdrif_dbg(2, ch, "bound asd: %s\n", asd->match.of.node->name);
> +	return 0;
> +}
> +
> +/* Sub-device registered notification callback */
> +static int rcar_drif_notify_complete(struct v4l2_async_notifier *notifier)
> +{
> +	struct rcar_drif_chan *ch =
> +		container_of(notifier, struct rcar_drif_chan, notifier);
> +	struct v4l2_subdev *sd, *tmp;
> +	int ret;
> +
> +	v4l2_ctrl_handler_init(&ch->ctrl_hdl, 10);
> +	ch->v4l2_dev.ctrl_handler = &ch->ctrl_hdl;
> +
> +	ret = v4l2_device_register_subdev_nodes(&ch->v4l2_dev);
> +	if (ret) {
> +		rdrif_err(ch, "failed register subdev nodes ret %d\n", ret);
> +		return ret;
> +	}
> +
> +	for_each_rcar_drif_subdev(sd, tmp, ch) {
> +		ret = v4l2_ctrl_add_handler(ch->v4l2_dev.ctrl_handler,
> +					    sd->ctrl_handler, NULL);
> +		if (ret) {
> +			rdrif_err(ch, "failed ctrl add hdlr ret %d\n", ret);
> +			return ret;
> +		}
> +	}
> +	rdrif_dbg(2, ch, "notify complete\n");
> +	return 0;
> +}
> +
> +/* Parse sub-devs (tuner) to find a matching device */
> +static int rcar_drif_parse_subdevs(struct device *dev,
> +				   struct v4l2_async_notifier *notifier)
> +{
> +	struct device_node *node = NULL;
> +
> +	notifier->subdevs = devm_kzalloc(dev, sizeof(*notifier->subdevs),
> +					 GFP_KERNEL);
> +	if (!notifier->subdevs)
> +		return -ENOMEM;
> +
> +	node = of_graph_get_next_endpoint(dev->of_node, node);

Do:

	if (!node)
		return 0;

And the remainder can be shifted one tab to the left.

> +	if (node) {
> +		struct rcar_drif_async_subdev *rsd;
> +
> +		rsd = devm_kzalloc(dev, sizeof(*rsd), GFP_KERNEL);
> +		if (!rsd) {
> +			of_node_put(node);
> +			return -ENOMEM;
> +		}
> +
> +		notifier->subdevs[notifier->num_subdevs] = &rsd->asd;
> +		rsd->asd.match.of.node = of_graph_get_remote_port_parent(node);
> +		of_node_put(node);
> +		if (!rsd->asd.match.of.node) {
> +			dev_warn(dev, "bad remote port parent\n");
> +			return -EINVAL;
> +		}
> +
> +		rsd->asd.match_type = V4L2_ASYNC_MATCH_OF;
> +		notifier->num_subdevs++;
> +	}
> +	return 0;
> +}
> +
> +/* SIRMDR1 configuration */
> +static int rcar_drif_validate_syncmd(struct rcar_drif_chan *ch, u32 val)
> +{
> +	if (val > 1) {
> +		dev_err(ch->dev, "invalid syncmd %u using L/R mode\n", val);
> +		return -EINVAL;
> +	}
> +
> +	ch->mdr1 &= ~(3 << 28);	/* Clear current settings */
> +	if (val == 0)
> +		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCMD_FRAME;
> +	else
> +		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCMD_LR;
> +	return 0;
> +}
> +
> +/* Get the dtdl or syncdl bits as in MSIOF */
> +static u32 rcar_drif_get_dtdl_or_syncdl_bits(u32 dtdl_or_syncdl)
> +{
> +	/*
> +	 * DTDL/SYNCDL bit	: dtdl/syncdl
> +	 * b'000		: 0
> +	 * b'001		: 100
> +	 * b'010		: 200
> +	 * b'011 (SYNCDL only)	: 300
> +	 * b'101		: 50
> +	 * b'110		: 150
> +	 */
> +	if (dtdl_or_syncdl % 100)
> +		return dtdl_or_syncdl / 100 + 5;
> +	else

Line can be dropped.

> +		return dtdl_or_syncdl / 100;
> +}
> +
> +static int rcar_drif_validate_dtdl_syncdl(struct rcar_drif_chan *ch)
> +{
> +	struct device_node *np = ch->dev->of_node;
> +	u32 dtdl = 100, syncdl = 0;
> +
> +	ch->mdr1 |= RCAR_DRIF_SIRMDR1_DTDL_1 | RCAR_DRIF_SIRMDR1_SYNCDL_0;
> +	of_property_read_u32(np, "renesas,dtdl", &dtdl);
> +	of_property_read_u32(np, "renesas,syncdl", &syncdl);
> +
> +	/* Sanity checks */
> +	if (dtdl > 200 || syncdl > 300) {
> +		dev_err(ch->dev, "invalid dtdl %u/syncdl %u\n", dtdl, syncdl);
> +		return -EINVAL;
> +	}
> +	if ((dtdl + syncdl) % 100) {
> +		dev_err(ch->dev, "sum of dtdl %u & syncdl %u not OK\n",
> +			dtdl, syncdl);
> +		return -EINVAL;
> +	}
> +	ch->mdr1 &= ~(7 << 20) & ~(7 << 16);	/* Clear current settings */
> +	ch->mdr1 |= rcar_drif_get_dtdl_or_syncdl_bits(dtdl) << 20;
> +	ch->mdr1 |= rcar_drif_get_dtdl_or_syncdl_bits(syncdl) << 16;
> +	return 0;
> +}
> +
> +static int rcar_drif_parse_properties(struct rcar_drif_chan *ch)
> +{
> +	struct device_node *np = ch->dev->of_node;
> +	u32 syncmd;
> +	int ret;
> +
> +	/* Set the defaults and check for overrides */
> +	ch->mdr1 = RCAR_DRIF_SIRMDR1_SYNCMD_LR;
> +	if (!of_property_read_u32(np, "renesas,syncmd", &syncmd)) {
> +		ret = rcar_drif_validate_syncmd(ch, syncmd);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (of_find_property(np, "renesas,lsb-first", NULL))
> +		ch->mdr1 |= RCAR_DRIF_SIRMDR1_LSB_FIRST;
> +	else
> +		ch->mdr1 |= RCAR_DRIF_SIRMDR1_MSB_FIRST;
> +
> +	if (of_find_property(np, "renesas,syncac-pol-high", NULL))
> +		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCAC_POL_HIGH;
> +	else
> +		ch->mdr1 |= RCAR_DRIF_SIRMDR1_SYNCAC_POL_LOW;
> +
> +	return rcar_drif_validate_dtdl_syncdl(ch);
> +}
> +
> +static u32 rcar_drif_enum_sub_channels(struct platform_device *pdev,
> +					struct platform_device *s_pdev[])
> +{
> +	struct device_node *s_np;
> +	u32 hw_schans_mask = 0;
> +	unsigned int i;
> +
> +	for (i = 0; i < RCAR_DRIF_MAX_SUBCHANS; i++) {
> +		s_np = of_parse_phandle(pdev->dev.of_node, "sub-channels", i);
> +		if (s_np && of_device_is_available(s_np)) {
> +			s_pdev[i] = of_find_device_by_node(s_np);
> +			if (s_pdev[i]) {
> +				hw_schans_mask |= BIT(i);
> +				dev_dbg(&s_pdev[i]->dev, "schan%u ok\n", i);
> +			}
> +		}
> +	}
> +	return hw_schans_mask;
> +}
> +
> +static int rcar_drif_probe(struct platform_device *pdev)
> +{
> +	struct platform_device *s_pdev[RCAR_DRIF_MAX_SUBCHANS];
> +	unsigned long hw_schans_mask;
> +	struct rcar_drif_chan *ch;
> +	unsigned int i;
> +	int ret;
> +
> +	/*
> +	 * Sub-channel resources are managed by the parent channel instance.
> +	 * The sub-channel instance helps only in registering with power domain
> +	 * to aid in run-time pm support
> +	 */
> +	if (!of_find_property(pdev->dev.of_node, "sub-channels", NULL))
> +		return 0;
> +
> +	/* Parent channel instance */
> +	hw_schans_mask = rcar_drif_enum_sub_channels(pdev, s_pdev);
> +	if (!hw_schans_mask) {
> +		dev_err(&pdev->dev, "no sub-channels enabled\n");
> +		return -ENODEV;
> +	}
> +
> +
> +	/* Reserve memory for driver structure */
> +	ch = devm_kzalloc(&pdev->dev, sizeof(*ch), GFP_KERNEL);
> +	if (!ch) {
> +		ret = PTR_ERR(ch);
> +		dev_err(&pdev->dev, "failed alloc drif context\n");
> +		return ret;
> +	}
> +	ch->dev = &pdev->dev;
> +
> +	/* Parse device tree optional properties */
> +	ret = rcar_drif_parse_properties(ch);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(ch->dev, "parsed mdr1 0x%08x\n", ch->mdr1);
> +
> +	/* Setup enabled sub-channels */
> +	for_each_rcar_drif_subchannel(i, &hw_schans_mask) {
> +		struct clk *clkp;
> +		struct resource	*res;
> +		void __iomem *base;
> +
> +		/* Peripheral clock */
> +		clkp = devm_clk_get(&s_pdev[i]->dev, "fck");
> +		if (IS_ERR(clkp)) {
> +			ret = PTR_ERR(clkp);
> +			dev_err(&s_pdev[i]->dev, "clk get failed (%d)\n", ret);
> +			return ret;
> +		}
> +
> +		/* Register map */
> +		res = platform_get_resource(s_pdev[i], IORESOURCE_MEM, 0);
> +		base = devm_ioremap_resource(&s_pdev[i]->dev, res);
> +		if (IS_ERR(base)) {
> +			ret = PTR_ERR(base);
> +			dev_err(&s_pdev[i]->dev, "ioremap failed (%d)\n", ret);
> +			return ret;
> +		}
> +
> +		/* Reserve memory for enabled sub-channel */
> +		ch->sch[i] = devm_kzalloc(&pdev->dev, sizeof(*ch->sch[i]),
> +					  GFP_KERNEL);
> +		if (!ch->sch[i]) {
> +			ret = PTR_ERR(ch);
> +			dev_err(&s_pdev[i]->dev, "failed alloc sub-channel\n");
> +			return ret;
> +		}
> +		ch->sch[i]->pdev = s_pdev[i];
> +		ch->sch[i]->clkp = clkp;
> +		ch->sch[i]->base = base;
> +		ch->sch[i]->num = i;
> +		ch->sch[i]->start = res->start;
> +		ch->sch[i]->parent = ch;
> +		ch->num_hw_schans++;
> +	}
> +	ch->hw_schans_mask = hw_schans_mask;
> +
> +	/* Validate any supported format for enabled sub-channels */
> +	ret = rcar_drif_set_default_format(ch);
> +	if (ret)
> +		return ret;
> +
> +	/* Set defaults */
> +	ch->num_hwbufs = RCAR_DRIF_DEFAULT_NUM_HWBUFS;
> +	ch->hwbuf_size = RCAR_DRIF_DEFAULT_HWBUF_SIZE;
> +
> +	mutex_init(&ch->v4l2_mutex);
> +	mutex_init(&ch->vb_queue_mutex);
> +	spin_lock_init(&ch->queued_bufs_lock);
> +	INIT_LIST_HEAD(&ch->queued_bufs);
> +
> +	/* Init videobuf2 queue structure */
> +	ch->vb_queue.type = V4L2_BUF_TYPE_SDR_CAPTURE;
> +	ch->vb_queue.io_modes = VB2_READ | VB2_MMAP | VB2_DMABUF;
> +	ch->vb_queue.drv_priv = ch;
> +	ch->vb_queue.buf_struct_size = sizeof(struct rcar_drif_frame_buf);
> +	ch->vb_queue.ops = &rcar_drif_vb2_ops;
> +	ch->vb_queue.mem_ops = &vb2_vmalloc_memops;
> +	ch->vb_queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +
> +	/* Init videobuf2 queue */
> +	ret = vb2_queue_init(&ch->vb_queue);
> +	if (ret) {
> +		dev_err(ch->dev, "could not initialize vb2 queue\n");
> +		return ret;
> +	}
> +
> +	/* Init video_device structure */
> +	ch->vdev = rcar_drif_vdev;

Don't embed video_device, use video_device_alloc instead. A lot of drivers
embed this, but it turns out not to be a good idea. So new drivers should
use video_device_alloc.

> +	ch->vdev.lock = &ch->v4l2_mutex;
> +	ch->vdev.queue = &ch->vb_queue;
> +	ch->vdev.queue->lock = &ch->vb_queue_mutex;
> +	ch->vdev.ctrl_handler = &ch->ctrl_hdl;
> +	video_set_drvdata(&ch->vdev, ch);
> +
> +	/* Register the v4l2_device */
> +	ret = v4l2_device_register(&pdev->dev, &ch->v4l2_dev);
> +	if (ret) {
> +		dev_err(ch->dev, "failed v4l2_device_register. ret %d\n", ret);
> +		return ret;
> +	}
> +
> +	ch->vdev.v4l2_dev = &ch->v4l2_dev;
> +
> +	/*
> +	 * Parse subdevs after v4l2_device_register because if the subdev
> +	 * is already probed, bound and complete will be called immediately
> +	 */
> +	ret = rcar_drif_parse_subdevs(&pdev->dev, &ch->notifier);
> +	if (ret)
> +		goto err_unreg_v4l2;
> +
> +	ch->notifier.bound = rcar_drif_notify_bound;
> +	ch->notifier.complete = rcar_drif_notify_complete;
> +
> +	/* Register notifier */
> +	ret = v4l2_async_notifier_register(&ch->v4l2_dev, &ch->notifier);
> +	if (ret < 0) {
> +		dev_err(ch->dev, "notifier registration failed\n");
> +		goto err_unreg_v4l2;
> +	}
> +
> +	/* Register SDR device */
> +	ret = video_register_device(&ch->vdev, VFL_TYPE_SDR, -1);
> +	if (ret) {
> +		dev_err(ch->dev, "failed video_register_device. ret %d\n", ret);
> +		goto err_unreg_notif;
> +	}
> +
> +	platform_set_drvdata(pdev, ch);
> +	dev_notice(ch->dev, "probed\n");
> +	return 0;
> +
> +err_unreg_notif:
> +	v4l2_async_notifier_unregister(&ch->notifier);
> +err_unreg_v4l2:
> +	v4l2_device_unregister(&ch->v4l2_dev);
> +	return ret;
> +}
> +
> +static int rcar_drif_remove(struct platform_device *pdev)
> +{
> +	struct rcar_drif_chan *ch = platform_get_drvdata(pdev);
> +
> +	if (!ch)
> +		return 0;
> +
> +	/* Parent channel instance */
> +	ch = platform_get_drvdata(pdev);
> +	v4l2_ctrl_handler_free(ch->v4l2_dev.ctrl_handler);
> +	v4l2_async_notifier_unregister(&ch->notifier);
> +	v4l2_device_unregister(&ch->v4l2_dev);
> +	video_unregister_device(&ch->vdev);
> +	dev_notice(ch->dev, "removed\n");
> +	return 0;
> +}
> +
> +static int __maybe_unused rcar_drif_suspend(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static int __maybe_unused rcar_drif_resume(struct device *dev)
> +{
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(rcar_drif_pm_ops, rcar_drif_suspend,
> +			 rcar_drif_resume);
> +
> +static const struct of_device_id rcar_drif_of_table[] = {
> +	{ .compatible = "renesas,rcar-gen3-drif" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, rcar_drif_of_table);
> +
> +#define RCAR_DRIF_DRV_NAME "rcar_drif"
> +static struct platform_driver rcar_drif_driver = {
> +	.driver = {
> +		.name = RCAR_DRIF_DRV_NAME,
> +		.of_match_table = of_match_ptr(rcar_drif_of_table),
> +		.pm = &rcar_drif_pm_ops,
> +		},
> +	.probe = rcar_drif_probe,
> +	.remove = rcar_drif_remove,
> +};
> +
> +module_platform_driver(rcar_drif_driver);
> +
> +MODULE_DESCRIPTION("Renesas R-Car Gen3 DRIF driver");
> +MODULE_ALIAS("platform:" RCAR_DRIF_DRV_NAME);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>");
> 

Regards,

	Hans

^ permalink raw reply

* RE: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Gabriele Paoloni @ 2016-11-11 13:39 UTC (permalink / raw)
  To: Arnd Bergmann, Gabriele Paoloni
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Yuanzhichang, mark.rutland-5wv7dgnIgG8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org,
	minyard-HInyCGIudOg@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org,
	John Garry, will.deacon-5wv7dgnIgG8@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, xuwei (O),
	Linuxarm, zourongrong-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	kantyzc-9Onoh4P/yGk@public.gmane.org
In-Reply-To: <10334260.ztLXZ2Oynd@wuerfel>

Hi Arnd

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd-r2nGTMty4D4@public.gmane.org]
> Sent: 10 November 2016 16:07
> To: Gabriele Paoloni
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; Yuanzhichang;
> mark.rutland-5wv7dgnIgG8@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org; minyard-HInyCGIudOg@public.gmane.org; linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org; John Garry; will.deacon-5wv7dgnIgG8@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; xuwei (O); Linuxarm; zourongrong-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org;
> robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; kantyzc-9Onoh4P/yGk@public.gmane.org; linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> catalin.marinas-5wv7dgnIgG8@public.gmane.org; olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org; liviu.dudau-5wv7dgnIgG8@public.gmane.org;
> bhelgaas@googl e.com; zhichang.yuan02-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> Subject: Re: [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on
> Hip06
> 
> On Thursday, November 10, 2016 3:36:49 PM CET Gabriele Paoloni wrote:
> >
> > Where should we get the range from? For LPC we know that it is going
> > Work on anything that is not used by PCI I/O space, and this is
> > why we use [0, PCIBIOS_MIN_IO]
> 
> It should be allocated the same way we allocate PCI config space
> segments. This is currently done with the io_range list in
> drivers/pci/pci.c, which isn't perfect but could be extended
> if necessary. Based on what others commented here, I'd rather
> make the differences between ISA/LPC and PCI I/O ranges smaller
> than larger.

I am not sure this would make sense...

IMHO all the mechanism around io_range_list is needed to provide the
"mapping" between I/O tokens and physical CPU addresses.

Currently the available tokens range from 0 to IO_SPACE_LIMIT.

As you know the I/O memory accessors operate on whatever
__of_address_to_resource sets into the resource (start, end).

With this special device in place we cannot know if a resource is
assigned with an I/O token or a physical address, unless we forbid
the I/O tokens to be in a specific range.

So this is why we are changing the offsets of all the functions
handling io_range_list (to make sure that a range is forbidden to
the tokens and is available to the physical addresses).

We have chosen this forbidden range to be [0, PCIBIOS_MIN_IO)
because this is the maximum physical I/O range that a non PCI device
can operate on and because we believe this does not impose much
restriction on the available I/O token range; that now is 
[PCIBIOS_MIN_IO, IO_SPACE_LIMIT].
So we believe that the chosen forbidden range can accommodate
any special ISA bus device with no much constraint on the rest
of I/O tokens...

> 
> > > Your current version has
> > >
> > >         if (arm64_extio_ops->pfout)                             \
> > >                 arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> > >                        addr, value, sizeof(type));             \
> > >
> > > Instead, just subtract the start of the range from the logical
> > > port number to transform it back into a bus-local port number:
> >
> > These accessors do not operate on IO tokens:
> >
> > If (arm64_extio_ops->start > addr || arm64_extio_ops->end < addr)
> > addr is not going to be an I/O token; in fact patch 2/3 imposes that
> > the I/O tokens will start at PCIBIOS_MIN_IO. So from 0 to
> PCIBIOS_MIN_IO
> > we have free physical addresses that the accessors can operate on.
> 
> Ah, I missed that part. I'd rather not use PCIBIOS_MIN_IO to refer to
> the logical I/O tokens, the purpose of that macro is really meant
> for allocating PCI I/O port numbers within the address space of
> one bus.

As I mentioned above, special devices operate on CPU addresses directly,
not I/O tokens. For them there is no way to distinguish....

> 
> Note that it's equally likely that whichever next platform needs
> non-mapped I/O access like this actually needs them for PCI I/O space,
> and that will use it on addresses registered to a PCI host bridge.

Ok so here you are talking about a platform that has got an I/O range
under the PCI host controller, right?
And this I/O range cannot be directly memory mapped but needs special
redirections for the I/O tokens, right?

In this scenario registering the I/O ranges with the forbidden range
implemented by the current patch would still allow to redirect I/O
tokens as long as arm64_extio_ops->start >= PCIBIOS_MIN_IO

So effectively the special PCI host controller
1) knows the physical range that needs special redirection
2) register such range
3) uses pci_pio_to_address() to retrieve the IO tokens for the
   special accessors
4) sets arm64_extio_ops->start/end to the IO tokens retrieved in 3)

So to be honest I think this patch can fit well both with
special PCI controllers that need I/O tokens redirection and with
special non-PCI controllers that need non-PCI I/O physical
address redirection...

Thanks (and sorry for the long reply but I didn't know how
to make the explanation shorter :) )

Gab

> 
> If we separate the two steps:
> 
> a) assign a range of logical I/O port numbers to a bus
> b) register a set of helpers for redirecting logical I/O
>    port to a helper function
> 
> then I think the code will get cleaner and more flexible.
> It should actually then be able to replace the powerpc
> specific implementation.
> 
> 	Arnd
--
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 2/5] media: i2c: max2175: Add MAX2175 support
From: Laurent Pinchart @ 2016-11-11 13:48 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Ramesh Shanmugasundaram, robh+dt, mark.rutland, mchehab,
	sakari.ailus, crope, chris.paterson2, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <46394837-c3f0-8487-750b-95dae7bcf859@xs4all.nl>

Hi Hans,

On Friday 11 Nov 2016 14:21:22 Hans Verkuil wrote:
> On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
> > This patch adds driver support for MAX2175 chip. This is Maxim
> > Integrated's RF to Bits tuner front end chip designed for software-defined
> > radio solutions. This driver exposes the tuner as a sub-device instance
> > with standard and custom controls to configure the device.
> > 
> > Signed-off-by: Ramesh Shanmugasundaram
> > <ramesh.shanmugasundaram@bp.renesas.com> ---
> > 
> >  .../devicetree/bindings/media/i2c/max2175.txt      |   61 +
> >  drivers/media/i2c/Kconfig                          |    4 +
> >  drivers/media/i2c/Makefile                         |    2 +
> >  drivers/media/i2c/max2175/Kconfig                  |    8 +
> >  drivers/media/i2c/max2175/Makefile                 |    4 +
> >  drivers/media/i2c/max2175/max2175.c                | 1558 +++++++++++++++
> >  drivers/media/i2c/max2175/max2175.h                |  108 ++
> >  7 files changed, 1745 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/media/i2c/max2175.txt
> >  create mode 100644 drivers/media/i2c/max2175/Kconfig
> >  create mode 100644 drivers/media/i2c/max2175/Makefile
> >  create mode 100644 drivers/media/i2c/max2175/max2175.c
> >  create mode 100644 drivers/media/i2c/max2175/max2175.h
> 
> <snip>
> 
> > diff --git a/drivers/media/i2c/max2175/max2175.c
> > b/drivers/media/i2c/max2175/max2175.c new file mode 100644
> > index 0000000..ec45b52
> > --- /dev/null
> > +++ b/drivers/media/i2c/max2175/max2175.c
> > @@ -0,0 +1,1558 @@
> 
> <snip>
> 
> > +/* Read/Write bit(s) on top of regmap */
> > +static int max2175_read(struct max2175 *ctx, u8 idx, u8 *val)
> > +{
> > +	u32 regval;
> > +	int ret = regmap_read(ctx->regmap, idx, &regval);
> > +
> > +	if (ret)
> > +		v4l2_err(ctx->client, "read ret(%d): idx 0x%02x\n", ret, idx);

By the way, I think I've seen a proposal to get rid of v4l2_err() in favour of 
dev_err(), was I dreaming or should this patch use dev_err() already ?

> > +
> > +	*val = regval;
> 
> Does regmap_read initialize regval even if it returns an error? If not,
> then I would initialize regval to 0 to prevent *val being uninitialized.

Better than that the error should be propagated to the caller and handled.

> > +	return ret;
> > +}

[snip]

> > +static int max2175_band_from_freq(u32 freq)
> > +{
> > +	if (freq >= 144000 && freq <= 26100000)
> > +		return MAX2175_BAND_AM;
> > +	else if (freq >= 65000000 && freq <= 108000000)
> > +		return MAX2175_BAND_FM;
> > +	else
> 
> No need for these 'else' keywords.

Indeed by in my opinion they improve readability :-)

> > +		return MAX2175_BAND_VHF;
> > +}

[snip]

> > +static const struct v4l2_ctrl_config max2175_na_rx_mode = {
> > +	.ops = &max2175_ctrl_ops,
> > +	.id = V4L2_CID_MAX2175_RX_MODE,
> > +	.name = "RX MODE",
> > +	.type = V4L2_CTRL_TYPE_MENU,
> > +	.max = ARRAY_SIZE(max2175_ctrl_na_rx_modes) - 1,
> > +	.def = 0,
> > +	.qmenu = max2175_ctrl_na_rx_modes,
> > +};
> 
> Please document all these controls better. This is part of the public API,
> so you need to give more information what this means exactly.

Should that go to Documentation/media/v4l-drivers/ ? If so "[PATCH v4 3/4] 
v4l: Add Renesas R-Car FDP1 Driver" can be used as an example.

[snip]

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH 3/5] media: Add new SDR formats SC16, SC18 & SC20
From: Laurent Pinchart @ 2016-11-11 13:48 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Ramesh Shanmugasundaram, robh+dt, mark.rutland, mchehab,
	sakari.ailus, crope, chris.paterson2, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <502a606c-2d66-4257-af17-7b7f35f2c839@xs4all.nl>

Hi Hans,

On Friday 11 Nov 2016 14:24:41 Hans Verkuil wrote:
> On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
> > This patch adds support for the three new SDR formats. These formats
> > were prefixed with "sliced" indicating I data constitutes the top half and
> > Q data constitutes the bottom half of the received buffer.
> 
> The standard terminology for video formats is "planar". I am leaning towards
> using that here as well.
> 
> Any opinions on this?

I like that as well, but I'm obviously biased :-)

> > V4L2_SDR_FMT_SCU16BE - 14-bit complex (I & Q) unsigned big-endian sample
> > inside 16-bit. V4L2 FourCC: SC16
> > 
> > V4L2_SDR_FMT_SCU18BE - 16-bit complex (I & Q) unsigned big-endian sample
> > inside 18-bit. V4L2 FourCC: SC18
> > 
> > V4L2_SDR_FMT_SCU20BE - 18-bit complex (I & Q) unsigned big-endian sample
> > inside 20-bit. V4L2 FourCC: SC20
> > 
> > Signed-off-by: Ramesh Shanmugasundaram
> > <ramesh.shanmugasundaram@bp.renesas.com> ---
> > 
> >  drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
> >  include/uapi/linux/videodev2.h       | 3 +++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c index 181381d..d36b386 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1207,6 +1207,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc
> > *fmt)> 
> >  	case V4L2_SDR_FMT_CS8:		descr = "Complex S8"; break;
> >  	case V4L2_SDR_FMT_CS14LE:	descr = "Complex S14LE"; break;
> >  	case V4L2_SDR_FMT_RU12LE:	descr = "Real U12LE"; break;
> > 
> > +	case V4L2_SDR_FMT_SCU16BE:	descr = "Sliced Complex U16BE"; break;
> > +	case V4L2_SDR_FMT_SCU18BE:	descr = "Sliced Complex U18BE"; break;
> > +	case V4L2_SDR_FMT_SCU20BE:	descr = "Sliced Complex U20BE"; break;
> > 
> >  	case V4L2_TCH_FMT_DELTA_TD16:	descr = "16-bit signed deltas"; break;
> >  	case V4L2_TCH_FMT_DELTA_TD08:	descr = "8-bit signed deltas"; break;
> >  	case V4L2_TCH_FMT_TU16:		descr = "16-bit unsigned touch data"; 
break;
> > 
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h index 4364ce6..34a9c30 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -666,6 +666,9 @@ struct v4l2_pix_format {
> > 
> >  #define V4L2_SDR_FMT_CS8          v4l2_fourcc('C', 'S', '0', '8') /*
> >  complex s8 */ #define V4L2_SDR_FMT_CS14LE       v4l2_fourcc('C', 'S',
> >  '1', '4') /* complex s14le */ #define V4L2_SDR_FMT_RU12LE      
> >  v4l2_fourcc('R', 'U', '1', '2') /* real u12le */> 
> > +#define V4L2_SDR_FMT_SCU16BE	  v4l2_fourcc('S', 'C', '1', '6') /* 
sliced
> > complex u16be */ +#define V4L2_SDR_FMT_SCU18BE	  v4l2_fourcc('S', 
'C',
> > '1', '8') /* sliced complex u18be */ +#define V4L2_SDR_FMT_SCU20BE	 
> > v4l2_fourcc('S', 'C', '2', '0') /* sliced complex u20be */> 
> >  /* Touch formats - used for Touch devices */
> >  #define V4L2_TCH_FMT_DELTA_TD16	v4l2_fourcc('T', 'D', '1', '6') /* 16-
bit
> >  signed deltas */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH 3/5] media: Add new SDR formats SC16, SC18 & SC20
From: Laurent Pinchart @ 2016-11-11 13:49 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Ramesh Shanmugasundaram, robh+dt, mark.rutland, mchehab,
	sakari.ailus, crope, chris.paterson2, geert+renesas, linux-media,
	devicetree, linux-renesas-soc
In-Reply-To: <502a606c-2d66-4257-af17-7b7f35f2c839@xs4all.nl>

Hi Hans,

On Friday 11 Nov 2016 14:24:41 Hans Verkuil wrote:
> On 11/09/2016 04:44 PM, Ramesh Shanmugasundaram wrote:
> > This patch adds support for the three new SDR formats. These formats
> > were prefixed with "sliced" indicating I data constitutes the top half and
> > Q data constitutes the bottom half of the received buffer.
> 
> The standard terminology for video formats is "planar". I am leaning towards
> using that here as well.

Another important question is whether we should introduce an SDR planar API. 
That's what we would like your opinion on.

> > V4L2_SDR_FMT_SCU16BE - 14-bit complex (I & Q) unsigned big-endian sample
> > inside 16-bit. V4L2 FourCC: SC16
> > 
> > V4L2_SDR_FMT_SCU18BE - 16-bit complex (I & Q) unsigned big-endian sample
> > inside 18-bit. V4L2 FourCC: SC18
> > 
> > V4L2_SDR_FMT_SCU20BE - 18-bit complex (I & Q) unsigned big-endian sample
> > inside 20-bit. V4L2 FourCC: SC20
> > 
> > Signed-off-by: Ramesh Shanmugasundaram
> > <ramesh.shanmugasundaram@bp.renesas.com> ---
> > 
> >  drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
> >  include/uapi/linux/videodev2.h       | 3 +++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c index 181381d..d36b386 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1207,6 +1207,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc
> > *fmt)> 
> >  	case V4L2_SDR_FMT_CS8:		descr = "Complex S8"; break;
> >  	case V4L2_SDR_FMT_CS14LE:	descr = "Complex S14LE"; break;
> >  	case V4L2_SDR_FMT_RU12LE:	descr = "Real U12LE"; break;
> > 
> > +	case V4L2_SDR_FMT_SCU16BE:	descr = "Sliced Complex U16BE"; break;
> > +	case V4L2_SDR_FMT_SCU18BE:	descr = "Sliced Complex U18BE"; break;
> > +	case V4L2_SDR_FMT_SCU20BE:	descr = "Sliced Complex U20BE"; break;
> > 
> >  	case V4L2_TCH_FMT_DELTA_TD16:	descr = "16-bit signed deltas"; break;
> >  	case V4L2_TCH_FMT_DELTA_TD08:	descr = "8-bit signed deltas"; break;
> >  	case V4L2_TCH_FMT_TU16:		descr = "16-bit unsigned touch data"; 
break;
> > 
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h index 4364ce6..34a9c30 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -666,6 +666,9 @@ struct v4l2_pix_format {
> > 
> >  #define V4L2_SDR_FMT_CS8          v4l2_fourcc('C', 'S', '0', '8') /*
> >  complex s8 */ #define V4L2_SDR_FMT_CS14LE       v4l2_fourcc('C', 'S',
> >  '1', '4') /* complex s14le */ #define V4L2_SDR_FMT_RU12LE      
> >  v4l2_fourcc('R', 'U', '1', '2') /* real u12le */> 
> > +#define V4L2_SDR_FMT_SCU16BE	  v4l2_fourcc('S', 'C', '1', '6') /* 
sliced
> > complex u16be */ +#define V4L2_SDR_FMT_SCU18BE	  v4l2_fourcc('S', 
'C',
> > '1', '8') /* sliced complex u18be */ +#define V4L2_SDR_FMT_SCU20BE	 
> > v4l2_fourcc('S', 'C', '2', '0') /* sliced complex u20be */> 
> >  /* Touch formats - used for Touch devices */
> >  #define V4L2_TCH_FMT_DELTA_TD16	v4l2_fourcc('T', 'D', '1', '6') /* 16-
bit
> >  signed deltas */

-- 
Regards,

Laurent Pinchart

^ 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