* [RFC PATCH 3/3] of: fix node traversing in of_dma_get_range
From: Oza Pawandeep via iommu @ 2017-03-25 5:31 UTC (permalink / raw)
To: Joerg Roedel, Robin Murphy
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490419893-5073-1-git-send-email-oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
it jumps to the parent node without examining the child node.
also with that, it throws "no dma-ranges found for node"
for pci dma-ranges.
this patch fixes device node traversing for dma-ranges.
Reviewed-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Oza Pawandeep <oza.oza-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 02b2903..3293d55 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -836,9 +836,6 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
while (1) {
naddr = of_n_addr_cells(node);
nsize = of_n_size_cells(node);
- node = of_get_next_parent(node);
- if (!node)
- break;
ranges = of_get_property(node, "dma-ranges", &len);
@@ -852,6 +849,10 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
*/
if (!ranges)
break;
+
+ node = of_get_next_parent(node);
+ if (!node)
+ break;
}
if (!ranges) {
--
1.9.1
^ permalink raw reply related
* RE: [RFC PATCH] iommu/dma/pci: account pci host bridge dma_mask for IOVA allocation
From: Oza Oza @ 2017-03-25 5:34 UTC (permalink / raw)
To: Robin Murphy
Cc: Joerg Roedel, linux-pci, iommu, linux-kernel, linux-arm-kernel,
devicetree, bcm-kernel-feedback-list
In-Reply-To: <bddd4c81-7d71-4207-4c5f-b632403daec9@arm.com>
Hi Robin,
I have made 3 separate patches now, which gives clear idea about the
changes.
we can have discussion there.
Regards,
Oza.
-----Original Message-----
From: Robin Murphy [mailto:robin.murphy@arm.com]
Sent: Monday, March 20, 2017 9:14 PM
To: Oza Oza
Cc: Joerg Roedel; linux-pci@vger.kernel.org;
iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
bcm-kernel-feedback-list@broadcom.com
Subject: Re: [RFC PATCH] iommu/dma/pci: account pci host bridge dma_mask for
IOVA allocation
On 20/03/17 08:57, Oza Oza wrote:
> + linux-pci
>
> Regards,
> Oza.
>
> -----Original Message-----
> From: Oza Pawandeep [mailto:oza.oza@broadcom.com]
> Sent: Friday, March 17, 2017 11:41 AM
> To: Joerg Roedel; Robin Murphy
> Cc: iommu@lists.linux-foundation.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> bcm-kernel-feedback-list@broadcom.com; Oza Pawandeep
> Subject: [RFC PATCH] iommu/dma: account pci host bridge dma_mask for
> IOVA allocation
>
> It is possible that PCI device supports 64-bit DMA addressing, and
> thus it's driver sets device's dma_mask to DMA_BIT_MASK(64), however
> PCI host bridge may have limitations on the inbound transaction
> addressing. As an example, consider NVME SSD device connected to
> iproc-PCIe controller.
>
> Currently, the IOMMU DMA ops only considers PCI device dma_mask when
> allocating an IOVA. This is particularly problematic on
> ARM/ARM64 SOCs where the IOMMU (i.e. SMMU) translates IOVA to PA for
> in-bound transactions only after PCI Host has forwarded these
> transactions on SOC IO bus. This means on such ARM/ARM64 SOCs the IOVA
> of in-bound transactions has to honor the addressing restrictions of the
> PCI Host.
>
> this patch is inspired by
> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1306545.ht
> ml http://www.spinics.net/lists/arm-kernel/msg566947.html
>
> but above inspiraiton solves the half of the problem.
> the rest of the problem is descrbied below, what we face on iproc
> based SOCs.
>
> current pcie frmework and of framework integration assumes dma-ranges
> in a way where memory-mapped devices define their dma-ranges.
> dma-ranges: (child-bus-address, parent-bus-address, length).
>
> but iproc based SOCs and even Rcar based SOCs has PCI world dma-ranges.
> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;
>
> of_dma_configure is specifically witten to take care of memory mapped
> devices.
> but no implementation exists for pci to take care of pcie based memory
> ranges.
> in fact pci world doesnt seem to define standard dma-ranges since
> there is an absense of the same, the dma_mask used to remain 32bit
> because of
> 0 size return (parsed by of_dma_configure())
>
> this patch also implements of_pci_get_dma_ranges to cater to pci world
> dma-ranges.
> so then the returned size get best possible (largest) dma_mask.
> for e.g.
> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>; we should get
> dev->coherent_dma_mask=0x7fffffffff.
>
> conclusion: there are following problems
> 1) linux pci and iommu framework integration has glitches with respect
> to dma-ranges
> 2) pci linux framework look very uncertain about dma-ranges, rather
> binding is not defined
> the way it is defined for memory mapped devices.
> rcar and iproc based SOCs use their custom one dma-ranges
> (rather can be standard)
> 3) even if in case of default parser of_dma_get_ranges,:
> it throws and erro"
> "no dma-ranges found for node"
> because of the bug which exists.
> following lines should be moved to the end of while(1)
> 839 node = of_get_next_parent(node);
> 840 if (!node)
> 841 break;
Right, having made sense of this and looked into things myself I think I
understand now; what this boils down to is that the existing implementation
of of_dma_get_range() expects always to be given a leaf device_node, and
doesn't cope with being given a device_node for the given device's parent
bus directly. That's really all there is; it's not specific to PCI (there
are other probeable and DMA-capable buses whose children aren't described in
DT, like the fsl-mc thing), and it definitely doesn't have anything to do
with IOMMUs.
Now, that's certainly something to fix, but AFAICS this patch doesn't do
that, only adds some PCI-specific code which is never called.
DMA mask inheritance for arm64 is another issue, which again is general, but
does tend to be more visible in the IOMMU case. That still needs some work
on the APCI side - all the DT-centric approaches so far either regress or at
best do nothing for ACPI. I've made a note to try to look into that soon,
but from what I recall I fear there is still an open question about what to
do for a default in the absence of IORT or _DMA (once the current assumption
that drivers can override our arbitrary default at will is closed down).
In the meantime, have you tried 4.11-rc1 or later on the affected system?
One of the ulterior motives behind 122fac030e91 was that in many cases it
also happens to paper over most versions of this problem for PCI devices,
and makes the IOMMU at least useable (on systems which don't need to
dma_map_*() vast amounts of RAM all at once) while we fix the underlying
things properly.
Robin.
> Reviewed-by: Anup Patel <anup.patel@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Oza Pawandeep <oza.oza@broadcom.com>
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index
> 8c7c244..20cfff7 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -217,6 +217,9 @@ config NEED_DMA_MAP_STATE config NEED_SG_DMA_LENGTH
> def_bool y
>
> +config ARCH_HAS_DMA_SET_COHERENT_MASK
> + def_bool y
> +
> config SMP
> def_bool y
>
> diff --git a/arch/arm64/include/asm/device.h
> b/arch/arm64/include/asm/device.h index 73d5bab..64b4dc3 100644
> --- a/arch/arm64/include/asm/device.h
> +++ b/arch/arm64/include/asm/device.h
> @@ -20,6 +20,7 @@ struct dev_archdata { #ifdef CONFIG_IOMMU_API
> void *iommu; /* private IOMMU data */
> #endif
> + u64 parent_dma_mask;
> bool dma_coherent;
> };
>
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index 81cdb2e..5845ecd 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -564,6 +564,7 @@ static void flush_page(struct device *dev, const
> void *virt, phys_addr_t phys)
> __dma_flush_area(virt, PAGE_SIZE);
> }
>
> +
> static void *__iommu_alloc_attrs(struct device *dev, size_t size,
> dma_addr_t *handle, gfp_t gfp,
> unsigned long attrs)
> @@ -795,6 +796,20 @@ static void __iommu_unmap_sg_attrs(struct device
> *dev,
> iommu_dma_unmap_sg(dev, sgl, nelems, dir, attrs); }
>
> +static int __iommu_set_dma_mask(struct device *dev, u64 mask) {
> + /* device is not DMA capable */
> + if (!dev->dma_mask)
> + return -EIO;
> +
> + if (mask > dev->archdata.parent_dma_mask)
> + mask = dev->archdata.parent_dma_mask;
> +
> + *dev->dma_mask = mask;
> +
> + return 0;
> +}
> +
> static const struct dma_map_ops iommu_dma_ops = {
> .alloc = __iommu_alloc_attrs,
> .free = __iommu_free_attrs,
> @@ -811,8 +826,21 @@ static void __iommu_unmap_sg_attrs(struct device
> *dev,
> .map_resource = iommu_dma_map_resource,
> .unmap_resource = iommu_dma_unmap_resource,
> .mapping_error = iommu_dma_mapping_error,
> + .set_dma_mask = __iommu_set_dma_mask,
> };
>
> +int dma_set_coherent_mask(struct device *dev, u64 mask) {
> + if (get_dma_ops(dev) == &iommu_dma_ops &&
> + mask > dev->archdata.parent_dma_mask)
> + mask = dev->archdata.parent_dma_mask;
> +
> + dev->coherent_dma_mask = mask;
> + return 0;
> +}
> +EXPORT_SYMBOL(dma_set_coherent_mask);
> +
> +
> /*
> * TODO: Right now __iommu_setup_dma_ops() gets called too early to do
> * everything it needs to - the device is only partially created and
> the @@ -975,6 +1003,8 @@ void arch_setup_dma_ops(struct device *dev,
> u64 dma_base, u64 size,
> if (!dev->dma_ops)
> dev->dma_ops = &swiotlb_dma_ops;
>
> + dev->archdata.parent_dma_mask = size - 1;
> +
> dev->archdata.dma_coherent = coherent;
> __iommu_setup_dma_ops(dev, dma_base, size, iommu); } diff --git
> a/drivers/of/of_pci.c b/drivers/of/of_pci.c index 0ee42c3..5804717
> 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -283,6 +283,51 @@ int of_pci_get_host_bridge_resources(struct
> device_node *dev,
> return err;
> }
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> +
> +int of_pci_get_dma_ranges(struct device_node *np, u64 *dma_addr, u64
> +*paddr, u64 *size) {
> + struct device_node *node = of_node_get(np);
> + int rlen, naddr, nsize, pna;
> + int ret = 0;
> + const int na = 3, ns = 2;
> + struct of_pci_range_parser parser;
> + struct of_pci_range range;
> +
> + if (!node)
> + return -EINVAL;
> +
> + parser.node = node;
> + parser.pna = of_n_addr_cells(node);
> + parser.np = parser.pna + na + ns;
> +
> + parser.range = of_get_property(node, "dma-ranges", &rlen);
> +
> + if (!parser.range) {
> + pr_debug("pcie device has no dma-ranges defined for
> node(%s)\n", np->full_name);
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + parser.end = parser.range + rlen / sizeof(__be32);
> +
> + /* how do we take care of multiple dma windows ?. */
> + for_each_of_pci_range(&parser, &range) {
> + *dma_addr = range.pci_addr;
> + *size = range.size;
> + *paddr = range.cpu_addr;
> + }
> +
> + pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> + *dma_addr, *paddr, *size);
> + *dma_addr = range.pci_addr;
> + *size = range.size;
> +
> +out:
> + of_node_put(node);
> + return ret;
> +
> +}
> +EXPORT_SYMBOL_GPL(of_pci_get_dma_ranges);
> #endif /* CONFIG_OF_ADDRESS */
>
> #ifdef CONFIG_PCI_MSI
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h index
> 0e0974e..907ace0 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -76,6 +76,7 @@ static inline void of_pci_check_probe_only(void) { }
> int of_pci_get_host_bridge_resources(struct device_node *dev,
> unsigned char busno, unsigned char bus_max,
> struct list_head *resources, resource_size_t *io_base);
> +int of_pci_get_dma_ranges(struct device_node *np, u64 *dma_addr, u64
> +*paddr, u64 *size);
> #else
> static inline int of_pci_get_host_bridge_resources(struct device_node
> *dev,
> unsigned char busno, unsigned char bus_max, @@
> -83,6 +84,11 @@ static inline int
> of_pci_get_host_bridge_resources(struct
> device_node *dev, {
> return -EINVAL;
> }
> +
> +static inline int of_pci_get_dma_ranges(struct device_node *np, u64
> +*dma_addr, u64 *paddr, u64 *size) {
> + return -EINVAL;
> +}
> #endif
>
> #if defined(CONFIG_OF) && defined(CONFIG_PCI_MSI)
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH 6/9] bus: brcmstb_gisb: correct support for 64-bit address output
From: Gregory Fong @ 2017-03-25 5:36 UTC (permalink / raw)
To: Doug Berger
Cc: Catalin Marinas, Rob Herring, Mark Rutland, Will Deacon,
Brian Norris, Florian Fainelli, bcm-kernel-feedback-list,
wangkefeng.wang-hv44wF8Li93QT0dZR+AlfA, james.morse-5wv7dgnIgG8,
vladimir.murzin-5wv7dgnIgG8, panand-H+wXaHxf7aLQT0dZR+AlfA,
andre.przywara-5wv7dgnIgG8, cmetcalf-VPRAkNaXOzVWk0Htik3J/w,
mingo-DgEjT+Ai2ygdnm+yROfE0A,
sandeepa.s.prabhu-Re5JQEeQqe8AvxtiuMwx3w,
shijie.huang-5wv7dgnIgG8, Linus Walleij,
treding-DDmLM1+adcrQT0dZR+AlfA, jonathanh-DDmLM1+adcrQT0dZR+AlfA,
Olof Johansson, mirza.krak-Re5JQEeQqe8AvxtiuMwx3w,
suzuki.poulose-5wv7dgnIgG8, bgolaszewski-rdvid1DuHRBWk0Htik3J/w,
horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20170324144632.5896-7-opendmb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, Mar 24, 2017 at 7:46 AM, Doug Berger <opendmb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> The GISB bus can support addresses beyond 32-bits. So this commit
> corrects support for reading a captured 64-bit address into a 64-bit
> variable by obtaining the high bits from the ARB_ERR_CAP_HI_ADDR
> register (when present) and then outputting the full 64-bit value.
>
> It also removes unused definitions.
>
> Fixes: 44127b771d9c ("bus: add Broadcom GISB bus arbiter timeout/error handler")
> Signed-off-by: Doug Berger <opendmb-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/bus/brcmstb_gisb.c | 36 ++++++++++++++++++++----------------
> 1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/bus/brcmstb_gisb.c b/drivers/bus/brcmstb_gisb.c
> [snip]
> @@ -119,6 +116,16 @@ static u32 gisb_read(struct brcmstb_gisb_arb_device *gdev, int reg)
> return ioread32(gdev->base + offset);
> }
>
> +static u64 gisb_read_address(struct brcmstb_gisb_arb_device *gdev)
> +{
> + u64 value;
> +
> + value = (u64)gisb_read(gdev, ARB_ERR_CAP_ADDR);
Unlike the one on the next line, this cast can be omitted.
> + value |= (u64)gisb_read(gdev, ARB_ERR_CAP_HI_ADDR) << 32;
> +
> + return value;
> +}
> [snip]
Acked-by: Gregory Fong <gregory.0xf0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/9] arm64: mm: install SError abort handler
From: Marc Zyngier @ 2017-03-25 10:06 UTC (permalink / raw)
To: Florian Fainelli
Cc: Mark Rutland, Doug Berger, catalin.marinas, robh+dt, will.deacon,
computersforpeace, gregory.0xf0, bcm-kernel-feedback-list,
wangkefeng.wang, james.morse, vladimir.murzin, panand,
andre.przywara, cmetcalf, mingo, sandeepa.s.prabhu, shijie.huang,
linus.walleij, treding, jonathanh, olof, mirza.krak,
suzuki.poulose, bgolaszewski, horms+renesas, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <b6090e54-a1f8-701a-6f86-6968d3effcf8@gmail.com>
On Fri, Mar 24 2017 at 07:02:05 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 03/24/2017 11:31 AM, Mark Rutland wrote:
>> Hi Florian,
>>
>> On Fri, Mar 24, 2017 at 10:53:48AM -0700, Florian Fainelli wrote:
>>> On 03/24/2017 10:35 AM, Mark Rutland wrote:
>>>> On Fri, Mar 24, 2017 at 09:48:40AM -0700, Doug Berger wrote:
>>>>> On 03/24/2017 08:16 AM, Mark Rutland wrote:
>>>>>> On Fri, Mar 24, 2017 at 07:46:26AM -0700, Doug Berger wrote:
>>>>
>>>>> If you would consider an alternative implementation where we scrap
>>>>> the SError handler (i.e. maintain the ugliness in our downstream
>>>>> kernel) in favor of a more gentle user mode crash on SError that
>>>>> allows the kernel the opportunity to service the interrupt for
>>>>> diagnostic purposes I could try to repackage that.
>>>>
>>>> If this is just for diagnostic purposes, I believe you can register a
>>>> panic notifier, which can then read from the bus. The panic will occur,
>>>> but you'll have the opportunity to log some information to dmesg.
>>>
>>> And crash the kernel? That sounds awful, FWIW the ARM/Linux kernel is
>>> able to recover just fine from user-space accessing e.g: invalid
>>> physical addresses in the GISB register space, bringing the same level
>>> of functionality to ARM64/Linux sounds reasonable to me.
>>
>> I disagree, given that:
>>
>> (a) You cannot determine the (HW) origin of the SError in an
>> architecturally portable way. i.e. when you take an SError, you have
>> no way of determining what asynchronous event caused it.
>>
>> (b) SError is effectively an edge-triggered interrupt for fatal system
>> errors (e.g. it may be triggered in resonse to ECC errors,
>> corruption detected in caches, etc). Even if you can determine that
>> the GISB triggered *an* SError, this does not tell you that this was
>> the *only* SError.
>
> Correct, which is why Doug's changes allow chaining of handlers.
>
>>
>> If you take an SError, something bad has already happened. Your data
>> may already have been corrupted, and worse, you don't know when or
>> where specifically this occurred (nor how many times).
>
> Sure, but that still allows you to send the correct signal to a faulting
> application (unless I am missing something here).
>
>>
>> (c) You cannot determine the (SW) origin of an SError without relying
>> upon implementation details. This cannot be written in a way that
>> does not rely on microarchitecture, integration, etc, and would need
>> to be updated for every future system with this misfeature.
>
> Which is exactly what is being done here, with the help of platform
> specific information (we would not load brcmstb_gisb.c if we were not on
> a platform where it makes sense to use that HW).
>
>>
>> (d) Even if you can determine the (SW) origin of an SError by relying on
>> IMPLEMENTATION DEFINED details, your handler needs to be intimately
>> familiar with the arch in question in order to attempt to recover.
>>
>> For example, the existing code tries to skip an ARM instruction in
>> some cases. For arm64 there are three cases that would need to be
>> handled (AArch64 A64, AArch32 A32/ARM, AArch32 T32/Thumb).
>>
>> Further, it appears to me that the existing code is broken given
>> that it doesn't handle Thumb, and given that it's skipping an
>> instruction in response to an asynchronous event -- i.e. some
>> arbitrary instruction after the one which triggered the abort.
>
> OK, that could presumably be fixed though.
>
>>
>> For better or worse, SError *must* be treated as fatal.
>
> I disagree here, since this is a platform specific SError exception that
> we can actually handle correctly there is a chance to actually not take
> down the system on something that can be made non fatal and informative
> at the same time.
>
>>
>> As Doug stated:
>>
>> The main benefit is to help debug user mode code that accidentally
>> maps a bad address since we would never make such an egregious error
>> in the kernel ;)
>>
>> This is just one of many ways a userspace application with direct HW
>> access can bring down the system. I see no reason to treat it any
>> differently, especially given the above points.
>
> Partially disagree, in the absence of a way to specifically deal with
> the exception, I would almost agree, but this is not the case here, we
> have a piece of HW that can help us locate the problem, display an
> informative message, and send a SIGBUS to the faulting application.
>
> Anyway, I won't argue much further than that, but I certainly don't
> think taking down an entire system is going to prove itself useful when
> you need to deploy such a kernel to hundreds of people who have no clue
> what so ever what their actual problem is in the first place. Taking a
> SIGBUS and printing a message can at least allow us to say: read more
> carefully, it say exactly what's wrong.
I think there is one point that hasn't been made in this discussion. You
seem to assume that an SError should be handled the same way on an ARMv8
system as you handle it on your ARMv7 platform.
In most cases, Linux on ARMv7 runs (for better or worse) in secure mode,
making it the only software agent capable of handling an Asynchronous
Abort. On v8, Linux runs in non-secure mode, and relies on secure
firmware for a large set of platform specific services (PM, CPU
bring-up...). Crucially, error handling is one of these services.
It is largely expected that an SError should be first taken to EL3
(SCR_EL3.EA being set), handled there by any platform-specific FW able
to triage and log the error, and could even be reported to EL1NS via
some standard mechanism. If the FW decides to reinject this SError to
the non-secure side, then this is bound to be fatal, because even the FW
couldn't handle it.
So my view is that you should move that kind of error handling to the
place where it actually belongs. It will give you the opportunity to
print out your debug messages if necessary, and leave the SError
handling in the kernel the way it should be: fatal.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply
* [PATCH] ARM: dts: clearfog: keep dts alphabetically ordered
From: Russell King @ 2017-03-25 12:53 UTC (permalink / raw)
To: Gregory Clement
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
Mark Rutland, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Keep the clearfog DTS file ordered alphabetically - Florian placed the
MDIO entry after pinctrl, which mis-orders the file.
Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
---
arch/arm/boot/dts/armada-388-clearfog.dts | 38 +++++++++++++++----------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/arch/arm/boot/dts/armada-388-clearfog.dts b/arch/arm/boot/dts/armada-388-clearfog.dts
index b9ad1641bcf3..e74904bcd0a0 100644
--- a/arch/arm/boot/dts/armada-388-clearfog.dts
+++ b/arch/arm/boot/dts/armada-388-clearfog.dts
@@ -183,25 +183,6 @@
};
};
-&pinctrl {
- clearfog_dsa0_clk_pins: clearfog-dsa0-clk-pins {
- marvell,pins = "mpp46";
- marvell,function = "ref";
- };
- clearfog_dsa0_pins: clearfog-dsa0-pins {
- marvell,pins = "mpp23", "mpp41";
- marvell,function = "gpio";
- };
- clearfog_spi1_cs_pins: spi1-cs-pins {
- marvell,pins = "mpp55";
- marvell,function = "spi1";
- };
- rear_button_pins: rear-button-pins {
- marvell,pins = "mpp34";
- marvell,function = "gpio";
- };
-};
-
&mdio {
status = "okay";
@@ -261,6 +242,25 @@
};
};
+&pinctrl {
+ clearfog_dsa0_clk_pins: clearfog-dsa0-clk-pins {
+ marvell,pins = "mpp46";
+ marvell,function = "ref";
+ };
+ clearfog_dsa0_pins: clearfog-dsa0-pins {
+ marvell,pins = "mpp23", "mpp41";
+ marvell,function = "gpio";
+ };
+ clearfog_spi1_cs_pins: spi1-cs-pins {
+ marvell,pins = "mpp55";
+ marvell,function = "spi1";
+ };
+ rear_button_pins: rear-button-pins {
+ marvell,pins = "mpp34";
+ marvell,function = "gpio";
+ };
+};
+
&spi1 {
/*
* Add SPI CS pins for clearfog:
--
2.7.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
* Re: [RESEND PATCH v2 00/53] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb
From: Masahiro Yamada @ 2017-03-25 14:40 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd, Laurent Monat, thorsten.christiansson, Enrico Jorns,
Artem Bityutskiy, Dinh Nguyen, Marek Vasut, Graham Moore,
David Woodhouse, Masami Hiramatsu, Chuanxiao Dong, Jassi Brar,
devicetree, Linux Kernel Mailing List, Brian Norris,
Richard Weinberger, Cyrille Pitchen, Rob Herring,
Mark Rutland <mark.>
In-Reply-To: <20170324211330.0e463621@bbrezillon>
Hi Boris,
2017-03-25 5:13 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
>>
>> Masahiro Yamada (53):
>> mtd: nand: allow to set only one of ECC size and ECC strength from DT
>> mtd: nand: use read_oob() instead of cmdfunc() for bad block check
>> mtd: nand: denali: remove unused CONFIG option and macros
>> mtd: nand: denali: remove redundant define of BANK(x)
>> mtd: nand: denali: remove more unused struct members
>> mtd: nand: denali: fix comment of denali_nand_info::flash_mem
>> mtd: nand: denali: consolidate INTR_STATUS__* and INTR_EN__* macros
>> mtd: nand: denali: introduce capability flag
>> mtd: nand: denali: use int where no reason to use fixed width variable
>> mtd: nand: denali: fix erased page checking
>> mtd: nand: denali: fix bitflips calculation in handle_ecc()
>> mtd: nand: denali: support HW_ECC_FIXUP capability
>> mtd: nand: denali_dt: enable HW_ECC_FIXUP for Altera SOCFPGA variant
>> mtd: nand: denali: support 64bit capable DMA engine
>> mtd: nand: denali_dt: remove dma-mask DT property
>> mtd: nand: denali_dt: use pdev instead of ofdev for platform_device
>> mtd: nand: denali: allow to override revision number
>> mtd: nand: denali: use nand_chip to hold frequently accessed data
>> mtd: nand: denali: call nand_set_flash_node() to set DT node
>> mtd: nand: denali: do not set mtd->name
>> mtd: nand: denali: move multi device fixup code to a helper function
>> mtd: nand: denali: simplify multi device fixup code
>> mtd: nand: denali: set DEVICES_CONNECTED 1 if not set
>> mtd: nand: denali: remove meaningless writes to read-only registers
>> mtd: nand: denali: remove unnecessary writes to ECC_CORRECTION
>> mtd: nand: denali: support 1024 byte ECC step size
>> mtd: nand: denali: avoid hard-coding ecc.strength and ecc.bytes
>> mtd: nand: denali: support "nand-ecc-strength" DT property
>> mtd: nand: denali: remove Toshiba and Hynix specific fixup code
>> mtd: nand: denali_dt: add compatible strings for UniPhier SoC variants
>> mtd: nand: denali: set NAND_ECC_CUSTOM_PAGE_ACCESS
>> mtd: nand: denali: do not propagate NAND_STATUS_FAIL to waitfunc()
>> mtd: nand: denali: use BIT() and GENMASK() for register macros
>> mtd: nand: denali: remove unneeded find_valid_banks()
>> mtd: nand: denali: handle timing parameters by setup_data_interface()
>> mtd: nand: denali: remove meaningless pipeline read-ahead operation
>> mtd: nand: denali: rework interrupt handling
>> mtd: nand: denali: fix NAND_CMD_STATUS handling
>> mtd: nand: denali: fix NAND_CMD_PARAM handling
>> mtd: nand: do not check R/B# for CMD_READID in nand_command(_lp)
>> mtd: nand: do not check R/B# for CMD_SET_FEATURES in nand_command(_lp)
>> mtd: nand: denali: switch over to cmd_ctrl instead of cmdfunc
>> mtd: nand: denali: fix bank reset function
>> mtd: nand: denali: use interrupt instead of polling for bank reset
>> mtd: nand: denali: propagate page to helpers via function argument
>> mtd: nand: denali: merge struct nand_buf into struct denali_nand_info
>> mtd: nand: denali: use flag instead of register macro for direction
>> mtd: nand: denali: fix raw and oob accessors for syndrome page layout
>> mtd: nand: denali: support hardware-assisted erased page detection
>> mtd: nand: allocate aligned buffers if NAND_OWN_BUFFERS is unset
>> mtd: nand: denali: skip driver internal bounce buffer when possible
>> mtd: nand: denali: use non-managed kmalloc() for DMA buffer
>> mtd: nand: denali: enable bad block table scan
>
> Applied patch 1 to 9, 40 and 41. I'll see what else I can apply so that
> you don't have to re-post the remaining 42 patches, but I fear some of
> them won't apply correctly without patch 10 on which I had comments.
Thanks!
This will be very helpful when sending the next version.
If you like, you can also apply the following 9 patches cleanly in this order.
18/53 mtd: nand: denali: use nand_chip to hold frequently accessed data
19/53 mtd: nand: denali: call nand_set_flash_node() to set DT node
20/53 mtd: nand: denali: do not set mtd->name
21/53 mtd: nand: denali: move multi device fixup code to a helper function
22/53 mtd: nand: denali: simplify multi device fixup code
23/53 mtd: nand: denali: set DEVICES_CONNECTED 1 if not set
24/53 mtd: nand: denali: remove meaningless writes to read-only registers
25/53 mtd: nand: denali: remove unnecessary writes to ECC_CORRECTION
36/53 mtd: nand: denali: remove meaningless pipeline read-ahead operation
These are less controversial, and have no dependency on 10/53-17/53.
50/53 "mtd: nand: allocate aligned buffers if NAND_OWN_BUFFERS is unset"
is a change to the NAND framework. (used as a pre-requisite for 51/53)
Surely no dependency on the others, but If you wait more for comments
from other developers, that's no problem.
I will update the commented parts,
and hopefully send v3 next week.
Thanks!
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH v5 0/8] Add dual-role OTG support for Allwinner H3
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
Allwinner H3 have a its USB PHY0 routed to two USB controllers: one is
a MUSB controller, which can work in peripheral mode, but works badly in
host mode (several hardware will fail on the MUSB controller, even connect
one MUSB controller in peripheral mode to another one in host mode cannot
work); the other is a pair of EHCI/OHCI controller, which can work only
in host mode, but have better compatibillity. The route is controlled in
a register, which we have set it to HCI only when we do not know about
it well.
Add support to route to the best controller according to current USB mode
(host/peripheral).
Note: Currently even if hardware only support hostmode, we should still
enable the MUSB controller, as it controls the USB mode. (Some this kind
of hardware can also work in peripheral mode by settings in the sysfs
node of MUSB, then connect it to another host via a USB Type-A to Type-A
cable.)
Patch 1 changes the device tree binding to include the "pmu0" for HCI pair.
Patch 2 and 3 are fixes for H3 PHY, in order to make MUSB really working
on H3.
Patch 4 adds support for auto routing of PHY0. It's currently only enabled
on H3, but it's easy to extend it to other SoCs which feature this
route control.
Patch 5 adds necessary device tree nodes to the H3/H5 DTSI file. Note: The
phy is not bind for OHCI/EHCI0, as OHCI/EHCI drivers will keep the VBUS
on. Only MUSB driver can properly handle a dual-role PHY.
Patch 6 enables USB OTG functionality on Orange Pi One board, which is
the only H3 board I have that have proper OTG function. It's easy to
enable OTG on other boards with their schematics.
Patch 7 enables USB OTG functionality on Orange Pi Zero board, as the
board cannot output power on Vbus, I only enabled peripheral mode by
default.
Patch 8 enables USB OTG funcionality on Orange Pi PC2 board, which is
the newly support H5 board, and have USB-related pins same as Orange
Pi One.
The USB PHY on V3s/A64 SoCs also feature this capability, and it will
be soon enabled on these SoCs after this patchset is merged.
Icenowy Zheng (8):
dt: bindings: add pmu0 regs for USB PHYs on Allwinner H3/V3s/A64
phy: sun4i-usb: change PHYCTL register clearing code
phy: sun4i-usb: add PHYCTL offset for H3 SoC
phy: sun4i-usb: support automatically switch PHY0 route to MUSB/HCI
ARM: sunxi: h3/h5: add usb_otg and OHCI/EHCI for usbc0 on H3/H5
ARM: sun8i: h3: enable USB OTG on Orange Pi One
ARM: sun8i: h2+: enable USB OTG for Orange Pi Zero board
arm64: allwinner: h5: enable USB OTG on Orange Pi PC 2 board
.../devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 20 +++++++-
arch/arm/boot/dts/sun8i-h3-orangepi-one.dts | 22 ++++++++-
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 32 ++++++++++++
.../boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts | 27 +++++++++-
drivers/phy/phy-sun4i-usb.c | 57 ++++++++++++++--------
6 files changed, 135 insertions(+), 24 deletions(-)
--
2.12.0
^ permalink raw reply
* [PATCH v5 1/8] dt: bindings: add pmu0 regs for USB PHYs on Allwinner H3/V3s/A64
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
Allwinner H3/V3s/A64 SoCs have a special USB PHY0 that can route to two
controllers: one is MUSB and the other is a EHCI/OHCI pair.
When it's routed to EHCI/OHCI pair, it will needs a "pmu0" regs to
tweak, like other EHCI/OHCI pairs in Allwinner SoCs.
Add this to the binding of USB PHYs on Allwinner H3/V3s/A64.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v5:
- Add Chen-Yu's ACK.
Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
index e42334258185..005bc22938ff 100644
--- a/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
+++ b/Documentation/devicetree/bindings/phy/sun4i-usb-phy.txt
@@ -15,6 +15,7 @@ Required properties:
- reg : a list of offset + length pairs
- reg-names :
* "phy_ctrl"
+ * "pmu0" for H3, V3s and A64
* "pmu1"
* "pmu2" for sun4i, sun6i or sun7i
- #phy-cells : from the generic phy bindings, must be 1
--
2.12.0
^ permalink raw reply related
* [PATCH v5 2/8] phy: sun4i-usb: change PHYCTL register clearing code
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
It seems that all SoCs with the PHYCTL register offset as 0x10 need the
PHYCTL register to be cleared before it's written.
Change PHYCTL register clearing code to judge whether clearing is needed
based on the PHYCTL offset.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
Changes in v5:
- Commit message fix.
drivers/phy/phy-sun4i-usb.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index a21b5f24a340..62b4d25448c6 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -188,10 +188,8 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,
spin_lock_irqsave(&phy_data->reg_lock, flags);
- if (phy_data->cfg->type == sun8i_a33_phy ||
- phy_data->cfg->type == sun50i_a64_phy ||
- phy_data->cfg->type == sun8i_v3s_phy) {
- /* A33 or A64 needs us to set phyctl to 0 explicitly */
+ if (phy_data->cfg->phyctl_offset == REG_PHYCTL_A33) {
+ /* SoCs newer than A33 need us to set phyctl to 0 explicitly */
writel(0, phyctl);
}
--
2.12.0
^ permalink raw reply related
* [PATCH v5 3/8] phy: sun4i-usb: add PHYCTL offset for H3 SoC
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
The config structure of H3 in phy-sun4i-usb driver have the PHYCTL
register offset missing.
Add it. From the BSP source code, we know that the offset should be
0x10.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
Changes in v5:
- Changed commit message.
drivers/phy/phy-sun4i-usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 62b4d25448c6..a650f283f6ff 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -821,6 +821,7 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
.num_phys = 4,
.type = sun8i_h3_phy,
.disc_thresh = 3,
+ .phyctl_offset = REG_PHYCTL_A33,
.dedicated_clocks = true,
.enable_pmu_unk1 = true,
};
--
2.12.0
^ permalink raw reply related
* [PATCH v5 4/8] phy: sun4i-usb: support automatically switch PHY0 route to MUSB/HCI
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
On newer Allwinner SoCs (H3 and after), the PHY0 node is routed to both
MUSB controller for peripheral and host support (the host support is
slightly broken), and a pair of EHCI/OHCI controllers, which provide a
better support for host mode.
Add support for automatically switch the route of PHY0 according to the
status of dr_mode and id det pin.
Only H3 have this function enabled in this patch, as further SoCs will
be tested later and then have it enabled.
As H5 is reusing the PHY driver of H3, this function is also enabled.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v3:
- Add Chen-Yu's ACK. (I made a patch 1 that changes dt binding)
Changes in v2:
- Re-route after force session end.
- Drop id_det based on role code in reroute function, as we already
properly set id_det in id_det getting function.
drivers/phy/phy-sun4i-usb.c | 50 ++++++++++++++++++++++++++++++---------------
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index a650f283f6ff..f86a2574b953 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -49,12 +49,14 @@
#define REG_PHYBIST 0x08
#define REG_PHYTUNE 0x0c
#define REG_PHYCTL_A33 0x10
-#define REG_PHY_UNK_H3 0x20
+#define REG_PHY_OTGCTL 0x20
#define REG_PMU_UNK1 0x10
#define PHYCTL_DATA BIT(7)
+#define OTGCTL_ROUTE_MUSB BIT(0)
+
#define SUNXI_AHB_ICHR8_EN BIT(10)
#define SUNXI_AHB_INCR4_BURST_EN BIT(9)
#define SUNXI_AHB_INCRX_ALIGN_EN BIT(8)
@@ -110,6 +112,7 @@ struct sun4i_usb_phy_cfg {
u8 phyctl_offset;
bool dedicated_clocks;
bool enable_pmu_unk1;
+ bool phy0_dual_route;
};
struct sun4i_usb_phy_data {
@@ -269,23 +272,16 @@ static int sun4i_usb_phy_init(struct phy *_phy)
writel(val & ~2, phy->pmu + REG_PMU_UNK1);
}
- if (data->cfg->type == sun8i_h3_phy) {
- if (phy->index == 0) {
- val = readl(data->base + REG_PHY_UNK_H3);
- writel(val & ~1, data->base + REG_PHY_UNK_H3);
- }
- } else {
- /* Enable USB 45 Ohm resistor calibration */
- if (phy->index == 0)
- sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
+ /* Enable USB 45 Ohm resistor calibration */
+ if (phy->index == 0)
+ sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
- /* Adjust PHY's magnitude and rate */
- sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
+ /* Adjust PHY's magnitude and rate */
+ sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
- /* Disconnect threshold adjustment */
- sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
- data->cfg->disc_thresh, 2);
- }
+ /* Disconnect threshold adjustment */
+ sun4i_usb_phy_write(phy, PHY_DISCON_TH_SEL,
+ data->cfg->disc_thresh, 2);
sun4i_usb_phy_passby(phy, 1);
@@ -484,6 +480,21 @@ static const struct phy_ops sun4i_usb_phy_ops = {
.owner = THIS_MODULE,
};
+static void sun4i_usb_phy0_reroute(struct sun4i_usb_phy_data *data, int id_det)
+{
+ u32 regval;
+
+ regval = readl(data->base + REG_PHY_OTGCTL);
+ if (id_det == 0) {
+ /* Host mode. Route phy0 to EHCI/OHCI */
+ regval &= ~OTGCTL_ROUTE_MUSB;
+ } else {
+ /* Peripheral mode. Route phy0 to MUSB */
+ regval |= OTGCTL_ROUTE_MUSB;
+ }
+ writel(regval, data->base + REG_PHY_OTGCTL);
+}
+
static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
{
struct sun4i_usb_phy_data *data =
@@ -544,6 +555,10 @@ static void sun4i_usb_phy0_id_vbus_det_scan(struct work_struct *work)
sun4i_usb_phy0_set_vbus_detect(phy0, 1);
mutex_unlock(&phy0->mutex);
}
+
+ /* Re-route PHY0 if necessary */
+ if (data->cfg->phy0_dual_route)
+ sun4i_usb_phy0_reroute(data, id_det);
}
if (vbus_notify)
@@ -698,7 +713,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
return PTR_ERR(phy->reset);
}
- if (i) { /* No pmu for usbc0 */
+ if (i || data->cfg->phy0_dual_route) { /* No pmu for musb */
snprintf(name, sizeof(name), "pmu%d", i);
res = platform_get_resource_byname(pdev,
IORESOURCE_MEM, name);
@@ -824,6 +839,7 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
.phyctl_offset = REG_PHYCTL_A33,
.dedicated_clocks = true,
.enable_pmu_unk1 = true,
+ .phy0_dual_route = true,
};
static const struct sun4i_usb_phy_cfg sun8i_v3s_cfg = {
--
2.12.0
^ permalink raw reply related
* [PATCH v5 5/8] ARM: sunxi: h3/h5: add usb_otg and OHCI/EHCI for usbc0 on H3/H5
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
Allwinner H3/H5 have a dual-routed USB PHY0 -- routed to either OHCI/EHCI
or MUSB controller.
Add device nodes for these controllers.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
---
Changes in v5:
- Change the size of &usb_otg address space from 0x0400 to 0x400.
Changes in v4:
- Add h5 to commit message.
Changes in v3:
- Add "h3:" to commit message.
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 2494ea063cd4..6640ebfa6419 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -144,6 +144,19 @@
#size-cells = <0>;
};
+ usb_otg: usb@01c19000 {
+ compatible = "allwinner,sun8i-h3-musb";
+ reg = <0x01c19000 0x400>;
+ clocks = <&ccu CLK_BUS_OTG>;
+ resets = <&ccu RST_BUS_OTG>;
+ interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "mc";
+ phys = <&usbphy 0>;
+ phy-names = "usb";
+ extcon = <&usbphy 0>;
+ status = "disabled";
+ };
+
usbphy: phy@01c19400 {
compatible = "allwinner,sun8i-h3-usb-phy";
reg = <0x01c19400 0x2c>,
@@ -176,6 +189,25 @@
#phy-cells = <1>;
};
+ ehci0: usb@01c1a000 {
+ compatible = "allwinner,sun8i-h3-ehci", "generic-ehci";
+ reg = <0x01c1a000 0x100>;
+ interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_EHCI0>, <&ccu CLK_BUS_OHCI0>;
+ resets = <&ccu RST_BUS_EHCI0>, <&ccu RST_BUS_OHCI0>;
+ status = "disabled";
+ };
+
+ ohci0: usb@01c1a400 {
+ compatible = "allwinner,sun8i-h3-ohci", "generic-ohci";
+ reg = <0x01c1a400 0x100>;
+ interrupts = <GIC_SPI 73 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_EHCI0>, <&ccu CLK_BUS_OHCI0>,
+ <&ccu CLK_USB_OHCI0>;
+ resets = <&ccu RST_BUS_EHCI0>, <&ccu RST_BUS_OHCI0>;
+ status = "disabled";
+ };
+
ehci1: usb@01c1b000 {
compatible = "allwinner,sun8i-h3-ehci", "generic-ehci";
reg = <0x01c1b000 0x100>;
--
2.12.0
^ permalink raw reply related
* [PATCH v5 6/8] ARM: sun8i: h3: enable USB OTG on Orange Pi One
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
Orange Pi One features a MicroUSB port that can work in both host mode
and peripheral mode.
When in host mode, its VBUS is controlled via a GPIO; when in peripheral
mode, its VBUS cannot be used to power up the board.
Add support for this port.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v5:
- Add Chen-Yu's ACK.
Changes in v3:
- Add "h3:" in commit message.
arch/arm/boot/dts/sun8i-h3-orangepi-one.dts | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
index ea8fd1325356..5fea430e0eb1 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
@@ -89,6 +89,10 @@
};
};
+&ehci0 {
+ status = "okay";
+};
+
&ehci1 {
status = "okay";
};
@@ -103,6 +107,10 @@
status = "okay";
};
+&ohci0 {
+ status = "okay";
+};
+
&ohci1 {
status = "okay";
};
@@ -126,6 +134,11 @@
};
};
+®_usb0_vbus {
+ gpio = <&r_pio 0 2 GPIO_ACTIVE_HIGH>; /* PL2 */
+ status = "okay";
+};
+
&uart0 {
pinctrl-names = "default";
pinctrl-0 = <&uart0_pins_a>;
@@ -150,7 +163,14 @@
status = "disabled";
};
+&usb_otg {
+ dr_mode = "otg";
+ status = "okay";
+};
+
&usbphy {
- /* USB VBUS is always on */
+ /* USB Type-A port's VBUS is always on */
+ usb0_id_det-gpios = <&pio 6 12 GPIO_ACTIVE_HIGH>; /* PG12 */
+ usb0_vbus-supply = <®_usb0_vbus>;
status = "okay";
};
--
2.12.0
^ permalink raw reply related
* [PATCH v5 7/8] ARM: sun8i: h2+: enable USB OTG for Orange Pi Zero board
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
Orange Pi Zero board features a USB OTG port, which has a ID pin, and
can be used to power up the board. However, even if the board is powered
via +5V pin in GPIO/expansion headers, the VBUS in the OTG port cannot
be powered up, thus it's impossible to use it in host mode with simple
OTG cables.
Add support for it in peripheral mode.
If someone really want to use it in host mode, the mode of PHY can be
switch via sysfs, then use a powered USB OTG cable or powered USB HUB to
power up external USB devices.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v5:
- Add Chen-Yu's ACK.
Changes in v3:
- Add "h2+:" in commit message.
arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
index 96c32293598a..9e8b082c134f 100644
--- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
+++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
@@ -95,6 +95,10 @@
};
};
+&ehci0 {
+ status = "okay";
+};
+
&ehci1 {
status = "okay";
};
@@ -131,6 +135,10 @@
bias-pull-up;
};
+&ohci0 {
+ status = "okay";
+};
+
&ohci1 {
status = "okay";
};
@@ -153,7 +161,17 @@
status = "disabled";
};
+&usb_otg {
+ dr_mode = "peripheral";
+ status = "okay";
+};
+
&usbphy {
- /* USB VBUS is always on */
+ /*
+ * USB Type-A port VBUS is always on. However, MicroUSB VBUS can only
+ * power up the board; when it's used as OTG port, this VBUS is
+ * always off even if the board is powered via GPIO pins.
+ */
status = "okay";
+ usb0_id_det-gpios = <&pio 6 12 GPIO_ACTIVE_HIGH>; /* PG12 */
};
--
2.12.0
^ permalink raw reply related
* [PATCH v5 8/8] arm64: allwinner: h5: enable USB OTG on Orange Pi PC 2 board
From: Icenowy Zheng @ 2017-03-25 14:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Rob Herring, Maxime Ripard, Chen-Yu Tsai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Icenowy Zheng
In-Reply-To: <20170325145015.22076-1-icenowy-ymACFijhrKM@public.gmane.org>
Orange Pi PC 2 board features a OTG port like the one on older H3 Orange
Pi's, with PG12 pin being the id det pin and PL2 being the vbus driver
pin.
Add support for it.
Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
Changes in v5:
- Add Chen-Yu's ACK.
.../boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts | 27 +++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
index 30639729920d..dfecc17dcc92 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts
@@ -90,6 +90,16 @@
gpios = <&r_pio 0 3 GPIO_ACTIVE_LOW>;
};
};
+
+ reg_usb0_vbus: usb0-vbus {
+ compatible = "regulator-fixed";
+ regulator-name = "usb0-vbus";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ enable-active-high;
+ gpio = <&r_pio 0 2 GPIO_ACTIVE_HIGH>; /* PL2 */
+ status = "okay";
+ };
};
&codec {
@@ -100,6 +110,10 @@
status = "okay";
};
+&ehci0 {
+ status = "okay";
+};
+
&ehci1 {
status = "okay";
};
@@ -127,6 +141,10 @@
status = "okay";
};
+&ohci0 {
+ status = "okay";
+};
+
&ohci1 {
status = "okay";
};
@@ -157,7 +175,14 @@
status = "disabled";
};
+&usb_otg {
+ dr_mode = "otg";
+ status = "okay";
+};
+
&usbphy {
- /* USB VBUS is always on */
+ /* USB Type-A ports' VBUS is always on */
+ usb0_id_det-gpios = <&pio 6 12 GPIO_ACTIVE_HIGH>; /* PG12 */
+ usb0_vbus-supply = <®_usb0_vbus>;
status = "okay";
};
--
2.12.0
^ permalink raw reply related
* Re: [PATCH v3 1/4] Documentation: dt-bindings: iio: Add max9611 ADC
From: Jonathan Cameron @ 2017-03-25 15:27 UTC (permalink / raw)
To: Jacopo Mondi, geert-Td1EMuHUCqxL1ZNQvxDV9g,
wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490369323-13866-2-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On 24/03/17 15:28, Jacopo Mondi wrote:
> Add device tree bindings documentation for Maxim max9611/max9612 current
> sense amplifier.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Fine with me, waiting for Mark or Rob to take a look though....
> ---
> .../devicetree/bindings/iio/adc/max9611.txt | 26 ++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/max9611.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/max9611.txt b/Documentation/devicetree/bindings/iio/adc/max9611.txt
> new file mode 100644
> index 0000000..8430d3b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/max9611.txt
> @@ -0,0 +1,26 @@
> +* Maxim max9611/max9612 current sense amplifier with 12-bits ADC interface
> +
> +Maxim max9611/max9612 is an high-side current sense amplifier with integrated
> +12-bits ADC communicating over I2c bus.
> +The device node for this driver shall be a child of a I2c controller.
> +
> +Required properties
> + - compatible: Should be "maxim,max9611" or "maxim,max9612"
> + - reg: The 7-bits long I2c address of the device
> + - shunt-resistor-uohm: Value, in uOhm, of the current sense shunt resistor
I'm fine with this being generic, but it makes this just non trivial enough that
I'll wait for Rob or Mark to have a chance to take a look at the binding.
> +
> +Example:
> +
> +&i2c4 {
> + csa: adc@7c {
> + compatible = "maxim,max9611";
> + reg = <0x7c>;
> +
> + shunt-resistor-uohm = <5000>;
> + };
> +};
> +
> +This device node describes a current sense amplifier sitting on I2c4 bus
> +with address 0x7c (read address is 0xf9, write address is 0xf8).
> +A sense resistor of 0,005 Ohm is installed between RS+ and RS- current-sensing
> +inputs.
>
--
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 v3 3/4] iio: adc: Add Maxim max9611 ADC driver
From: Jonathan Cameron @ 2017-03-25 15:45 UTC (permalink / raw)
To: Jacopo Mondi, geert-Td1EMuHUCqxL1ZNQvxDV9g,
wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490369323-13866-4-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On 24/03/17 15:28, Jacopo Mondi wrote:
> Add iio driver for Maxim max9611 and max9612 current-sense amplifiers
> with 12-bits ADC interface.
>
> Datasheet publicly available at:
> https://datasheets.maximintegrated.com/en/ds/MAX9611-MAX9612.pdf
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
A few more little things inline. Coming together nicely.
The channel set here is just odd enough that it might aid review to have
a quick listing of the resulting sysfs entries. One or two authors do
this an it is always useful for a quick sanity check.
Perhaps even a set of typical values. Put this below the --- as we don't
need it in the git history, only to assist lazy reviewers like me ;)
Thanks,
Jonathan
> ---
> drivers/iio/adc/Kconfig | 10 +
> drivers/iio/adc/Makefile | 1 +
> drivers/iio/adc/max9611.c | 590 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 601 insertions(+)
> create mode 100644 drivers/iio/adc/max9611.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index dedae7a..82f2e7b8 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -354,6 +354,16 @@ config MAX1363
> To compile this driver as a module, choose M here: the module will be
> called max1363.
>
> +config MAX9611
> + tristate "Maxim max9611/max9612 ADC driver"
> + depends on I2C
> + help
> + Say yes here to build support for Maxim max9611/max9612 current sense
> + amplifier with 12-bits ADC interface.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called max9611.
> +
> config MCP320X
> tristate "Microchip Technology MCP3x01/02/04/08"
> depends on SPI
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index d001262..149f979 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_LTC2485) += ltc2485.o
> obj-$(CONFIG_MAX1027) += max1027.o
> obj-$(CONFIG_MAX11100) += max11100.o
> obj-$(CONFIG_MAX1363) += max1363.o
> +obj-$(CONFIG_MAX9611) += max9611.o
> obj-$(CONFIG_MCP320X) += mcp320x.o
> obj-$(CONFIG_MCP3422) += mcp3422.o
> obj-$(CONFIG_MEDIATEK_MT6577_AUXADC) += mt6577_auxadc.o
> diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c
> new file mode 100644
> index 0000000..61566ec
> --- /dev/null
> +++ b/drivers/iio/adc/max9611.c
> @@ -0,0 +1,590 @@
> +/*
> + * iio/adc/max9611.c
> + *
> + * Maxim max9611/max9612 high side current sense amplifier with
> + * 12-bit ADC interface.
> + *
> + * Copyright (C) 2017 Jacopo Mondi
> + *
> + * 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 driver supports input common-mode voltage, current-sense
> + * amplifier with programmable gains and die temperature reading from
> + * Maxim max9611/max9612.
> + *
> + * Op-amp, analog comparator, and watchdog functionalities are not
> + * supported by this driver.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/module.h>
> +
> +#define DRIVER_NAME "max9611"
> +
> +/* max9611 register addresses */
> +#define MAX9611_REG_CSA_DATA 0x00
> +#define MAX9611_REG_RS_DATA 0x02
> +#define MAX9611_REG_TEMP_DATA 0x08
> +#define MAX9611_REG_CTRL1 0x0a
> +#define MAX9611_REG_CTRL2 0x0b
> +
> +/* max9611 REG1 mux configuration options */
> +#define MAX9611_MUX_MASK 0x07
> +#define MAX9611_MUX_SENSE_1x 0x00
> +#define MAX9611_MUX_SENSE_4x 0x01
> +#define MAX9611_MUX_SENSE_8x 0x02
> +#define MAX9611_INPUT_VOLT 0x03
> +#define MAX9611_MUX_TEMP 0x06
> +
> +/* max9611 voltage (both csa and input) helper macros */
> +#define MAX9611_VOLTAGE_SHIFT 0x04
> +#define MAX9611_VOLTAGE_RAW(_r) ((_r) >> MAX9611_VOLTAGE_SHIFT)
> +
> +/*
> + * max9611 current sense amplifier voltage output:
> + * LSB and offset values depends on selected gain (1x, 4x, 8x)
> + *
> + * GAIN LSB (nV) OFFSET (LSB steps)
> + * 1x 107500 1
> + * 4x 26880 1
> + * 8x 13440 3
> + *
> + * The complete formula to calculate current sense voltage is:
> + * (((adc_read >> 4) - offset) / ((1 / LSB) * 10^-3)
> + */
> +#define CSA_VOLT_1x_LSB_nV 107500
> +#define CSA_VOLT_4x_LSB_nV 26880
> +#define CSA_VOLT_8x_LSB_nV 13440
> +
> +#define CSA_VOLT_1x_OFFS_RAW 1
> +#define CSA_VOLT_4x_OFFS_RAW 1
> +#define CSA_VOLT_8x_OFFS_RAW 3
> +
> +/*
> + * max9611 common input mode (CIM): LSB is 14mV, with 14mV offset at 25 C
> + *
> + * The complete formula to calculate input common voltage is:
> + * (((adc_read >> 4) * 1000) - offset) / (1 / 14 * 1000)
> + */
> +#define CIM_VOLTAGE_LSB_mV 14
> +#define CIM_VOLTAGE_OFFSET_RAW 1
> +
> +/*
> + * max9611 temperature reading: LSB is 0.48 degrees Celsius
> + *
> + * The complete formula to calculate temperature is:
> + * ((adc_read >> 7) * 1000) / (1 / 0.48 * 1000)
> + */
I'd prefer these defines to be prefixed with MAX9611_
Easiest to just do the lot. Some of these are 'standard' enough
the might well clash with something that turns up in an included header
at somepoint in the future.
> +#define TEMP_SHIFT 0x07
> +#define TEMP_MAX_RAW_POS 0x7f80
> +#define TEMP_MAX_RAW_NEG 0xff80
> +#define TEMP_MIN_RAW_NEG 0xd980
> +#define TEMP_MASK ((1 << TEMP_SHIFT) - 1)
> +#define TEMP_RAW(_r) ((_r) >> TEMP_SHIFT)
> +#define TEMP_LSB_mC 480
> +#define TEMP_SCALE_NUM 1000
> +#define TEMP_SCALE_DIV 2083
> +
> +struct max9611_dev {
> + struct device *dev;
> + struct i2c_client *i2c_client;
> + struct mutex lock;
> + unsigned int shunt_resistor_uohm;
> +};
> +
> +enum max9611_conf_ids {
> + CONF_SENSE_1x,
> + CONF_SENSE_4x,
> + CONF_SENSE_8x,
> + CONF_IN_VOLT,
> + CONF_TEMP,
> +};
> +
> +/**
> + * max9611_mux_conf - associate ADC mux configuration with register address
> + * where data shall be read from
> + */
> +static unsigned int max9611_mux_conf[][2] = {
> + /* CONF_SENSE_1x */
> + { MAX9611_MUX_SENSE_1x, MAX9611_REG_CSA_DATA },
> + /* CONF_SENSE_4x */
> + { MAX9611_MUX_SENSE_4x, MAX9611_REG_CSA_DATA },
> + /* CONF_SENSE_8x */
> + { MAX9611_MUX_SENSE_8x, MAX9611_REG_CSA_DATA },
> + /* CONF_IN_VOLT */
> + { MAX9611_INPUT_VOLT, MAX9611_REG_RS_DATA },
> + /* CONF_TEMP */
> + { MAX9611_MUX_TEMP, MAX9611_REG_TEMP_DATA },
> +};
> +
> +enum max9611_csa_gain {
> + CSA_GAIN_1x,
> + CSA_GAIN_4x,
> + CSA_GAIN_8x,
> +};
> +
> +enum max9611_csa_gain_params {
> + CSA_GAIN_LSB_nV,
> + CSA_GAIN_OFFS_RAW,
> +};
> +
> +/**
> + * max9611_csa_gain_conf - associate gain multiplier with LSB and
> + * offset values.
> + *
> + * Group together parameters associated with configurable gain
> + * on current sense amplifier path to ADC interface.
> + * Current sense read routine adjusts gain until it gets a meaningful
> + * value; use this structure to retrieve the correct LSB and offset values.
> + */
> +static unsigned int max9611_gain_conf[][2] = {
> + { /* [0] CSA_GAIN_1x */
> + CSA_VOLT_1x_LSB_nV,
> + CSA_VOLT_1x_OFFS_RAW,
> + },
> + { /* [1] CSA_GAIN_4x */
> + CSA_VOLT_4x_LSB_nV,
> + CSA_VOLT_4x_OFFS_RAW,
> + },
> + { /* [2] CSA_GAIN_8x */
> + CSA_VOLT_8x_LSB_nV,
> + CSA_VOLT_8x_OFFS_RAW,
> + },
> +};
> +
> +enum max9611_chan_addrs {
> + MAX9611_CHAN_VOLTAGE_INPUT,
> + MAX9611_CHAN_VOLTAGE_SENSE,
> + MAX9611_CHAN_TEMPERATURE,
> + MAX9611_CHAN_CURRENT_LOAD,
> + MAX9611_CHAN_POWER_LOAD,
> +};
> +
> +static struct iio_chan_spec max9611_channels[] = {
> + {
> + .type = IIO_TEMP,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .address = MAX9611_CHAN_TEMPERATURE,
> + },
> + {
> + .type = IIO_VOLTAGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_OFFSET),
> + .address = MAX9611_CHAN_VOLTAGE_INPUT,
> + .indexed = 1,
> + .channel = 1,
> + },
> + {
> + .type = IIO_VOLTAGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .address = MAX9611_CHAN_VOLTAGE_SENSE,
> + .indexed = 1,
> + .channel = 0,
Unusual to have the channels in here other than in channel order...
> + },
> + {
> + .type = IIO_CURRENT,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .address = MAX9611_CHAN_CURRENT_LOAD,
> + },
> + {
> + .type = IIO_POWER,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> + .address = MAX9611_CHAN_POWER_LOAD
> + },
> +};
> +
> +/**
> + * max9611_read_single() - read a single vale from ADC interface
value
> + *
> + * Data registers are 16 bit long, spread between two 8 bit registers
> + * with consecutive addresses.
> + * Configure ADC mux first, then read register at address "reg_addr".
> + * The smbus_read_word routine asks for 16 bits and the ADC is kind enough
> + * to return values from "reg_addr" and "reg_addr + 1" consecutively.
> + *
> + * @max9611: max9611 device
> + * @selector: index for mux and register configuration
> + * @raw_val: the value returned from ADC
> + */
> +static int max9611_read_single(struct max9611_dev *max9611,
> + enum max9611_conf_ids selector,
> + u16 *raw_val)
> +{
> + int ret;
> +
> + u8 mux_conf = max9611_mux_conf[selector][0] & MAX9611_MUX_MASK;
> + u8 reg_addr = max9611_mux_conf[selector][1];
> +
> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> + MAX9611_REG_CTRL1, mux_conf);
> + if (ret) {
> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> + MAX9611_REG_CTRL1, mux_conf);
> + return ret;
> + }
> +
> + /*
> + * need a delay here to make register configuration
> + * stabilize. 1 msec at least, from empirical testing.
> + */
> + usleep_range(1000, 2000);
> +
> + ret = i2c_smbus_read_word_swapped(max9611->i2c_client, reg_addr);
> + if (ret < 0) {
> + dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
> + reg_addr);
> + return ret;
> + }
> + *raw_val = ret;
> +
> + return 0;
> +}
> +
> +/**
> + * max9611_read_csa_voltage() - read current sense amplifier output voltage
> + *
> + * Current sense amplifier output voltage is read through a configurable
> + * 1x, 4x or 8x gain.
> + * Start with plain 1x gain, and adjust gain control properly until a
> + * meaningful value is read from ADC output.
> + *
> + * @max9611: max9611 device
> + * @adc_raw: raw value read from ADC output
> + * @csa_gain: gain configuration option selector
> + */
> +static int max9611_read_csa_voltage(struct max9611_dev *max9611,
> + u16 *adc_raw,
> + enum max9611_csa_gain *csa_gain)
> +{
> + enum max9611_conf_ids gain_selectors[] = {
> + CONF_SENSE_1x,
> + CONF_SENSE_4x,
> + CONF_SENSE_8x
> + };
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < ARRAY_SIZE(gain_selectors); ++i) {
> + ret = max9611_read_single(max9611, gain_selectors[i], adc_raw);
> + if (ret)
> + return ret;
> +
> + if (*adc_raw > 0) {
> + *csa_gain = gain_selectors[i];
> + return 0;
> + }
> + }
> +
> + return -EIO;
> +}
> +
> +static int max9611_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max9611_dev *dev = iio_priv(indio_dev);
> + enum max9611_csa_gain gain_selector;
> + unsigned int *csa_gain;
> + u16 adc_data;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + mutex_lock(&dev->lock);
> +
> + switch (chan->address) {
> + case MAX9611_CHAN_TEMPERATURE:
> + ret = max9611_read_single(dev, CONF_TEMP,
> + &adc_data);
> + if (ret)
I'm not personally keen on jumping out of deep indentations like this
just save on repeating one line. I'd pull the unlock back here and return
directly as I feel it'll improve readability.
Actually come to think of it, why does the lock need to be held for
the next line anyway? adc_data is on the stack so doesn't matter if we
have concurrent readers, once the i2c transaction is finished.
Just unlock before checking ret.
> + goto unlock_fail;
> +
> + *val = TEMP_RAW(adc_data);
> +
> + mutex_unlock(&dev->lock);
> + return IIO_VAL_INT;
> +
> + case MAX9611_CHAN_VOLTAGE_INPUT:
> + ret = max9611_read_single(dev, CONF_IN_VOLT,
> + &adc_data);
> + if (ret)
> + goto unlock_fail;
> +
> + *val = MAX9611_VOLTAGE_RAW(adc_data);
> +
> + mutex_unlock(&dev->lock);
> + return IIO_VAL_INT;
> + }
> +
> + case IIO_CHAN_INFO_OFFSET:
> + switch (chan->address) {
> + case MAX9611_CHAN_VOLTAGE_INPUT:
> + *val = CIM_VOLTAGE_OFFSET_RAW;
> +
> + return IIO_VAL_INT;
> + }
> +
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->address) {
> + case MAX9611_CHAN_TEMPERATURE:
> + *val = TEMP_SCALE_NUM;
> + *val2 = TEMP_SCALE_DIV;
> +
> + return IIO_VAL_FRACTIONAL;
> +
> + case MAX9611_CHAN_VOLTAGE_INPUT:
> + *val = CIM_VOLTAGE_LSB_mV;
> + return IIO_VAL_INT;
> + }
> +
> + case IIO_CHAN_INFO_PROCESSED:
> + mutex_lock(&dev->lock);
> +
> + switch (chan->address) {
> + case MAX9611_CHAN_VOLTAGE_SENSE:
> + /*
> + * processed (mV): (raw - offset) * LSB (nV) / 10^6
> + *
> + * Even if max9611 can output raw csa voltage readings,
> + * use a produced value as scale depends on gain.
> + */
> + ret = max9611_read_csa_voltage(dev, &adc_data,
> + &gain_selector);
> + if (ret)
> + goto unlock_fail;
> +
> + csa_gain = max9611_gain_conf[gain_selector];
> +
> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> + csa_gain[CSA_GAIN_LSB_nV];
> + *val2 = 1000000;
> +
> + mutex_unlock(&dev->lock);
> + return IIO_VAL_FRACTIONAL;
> +
> + case MAX9611_CHAN_CURRENT_LOAD:
> + /* processed (mA): Vcsa (nV) / Rshunt (uOhm) */
> + ret = max9611_read_csa_voltage(dev, &adc_data,
> + &gain_selector);
> + if (ret)
> + goto unlock_fail;
> +
> + csa_gain = max9611_gain_conf[gain_selector];
> +
> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> + csa_gain[CSA_GAIN_LSB_nV];
> + *val2 = dev->shunt_resistor_uohm;
> +
> + mutex_unlock(&dev->lock);
> + return IIO_VAL_FRACTIONAL;
> +
> + case MAX9611_CHAN_POWER_LOAD:
> + /*
> + * processed (mW): Vin (mV) * Vcsa (uV) /
> + * Rshunt (uOhm)
> + */
> + ret = max9611_read_single(dev, CONF_IN_VOLT,
> + &adc_data);
> + if (ret)
> + goto unlock_fail;
> +
> + adc_data -= CIM_VOLTAGE_OFFSET_RAW;
> + *val = MAX9611_VOLTAGE_RAW(adc_data) *
> + CIM_VOLTAGE_LSB_mV;
> +
> + ret = max9611_read_csa_voltage(dev, &adc_data,
> + &gain_selector);
> + if (ret)
> + goto unlock_fail;
> +
> + csa_gain = max9611_gain_conf[gain_selector];
> +
> + /* divide by 10^3 here to avoid 32bit overflow */
> + adc_data -= csa_gain[CSA_GAIN_OFFS_RAW];
> + *val *= MAX9611_VOLTAGE_RAW(adc_data) *
> + csa_gain[CSA_GAIN_LSB_nV] / 1000;
> + *val2 = dev->shunt_resistor_uohm;
> +
> + mutex_unlock(&dev->lock);
> + return IIO_VAL_FRACTIONAL;
> + }
> + }
> +
> + ret = -EINVAL;
> +
> +unlock_fail:
> + mutex_unlock(&dev->lock);
> + return ret;
> +
> +}
> +
> +static ssize_t max9611_shunt_resistor_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct max9611_dev *max9611 = iio_priv(dev_to_iio_dev(dev));
> +
> + return sprintf(buf, "%d\n", max9611->shunt_resistor_uohm);
> +}
> +
> +static IIO_DEVICE_ATTR(in_shunt_resistor_power, 0444,
> + max9611_shunt_resistor_show, NULL, 0);
> +static IIO_DEVICE_ATTR(in_shunt_resistor_current, 0444,
> + max9611_shunt_resistor_show, NULL, 0);
> +
> +static struct attribute *max9611_attributes[] = {
> + &iio_dev_attr_in_shunt_resistor_power.dev_attr.attr,
> + &iio_dev_attr_in_shunt_resistor_current.dev_attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group max9611_attribute_group = {
> + .attrs = max9611_attributes,
> +};
> +
> +static const struct iio_info indio_info = {
> + .driver_module = THIS_MODULE,
> + .read_raw = max9611_read_raw,
> + .attrs = &max9611_attribute_group,
> +};
> +
> +static int max9611_init(struct max9611_dev *max9611)
> +{
> + struct i2c_client *client = max9611->i2c_client;
> + u16 regval;
> + int ret;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_WRITE_BYTE |
> + I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> + dev_err(max9611->dev,
> + "No smbus support in I2c adapter: aborting probe.\n");
This isn't necessarily an accurate message. I2c adapter might support
smbus_read_byte only rather than word reads for example.
Maybe make it more explict as to what we need?
> + return -EINVAL;
> + }
> +
> + /* Configure MUX to read temperature */
> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> + MAX9611_REG_CTRL1, MAX9611_MUX_TEMP);
> + if (ret) {
> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> + MAX9611_REG_CTRL1, MAX9611_MUX_TEMP);
> + return ret;
> + }
> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> + MAX9611_REG_CTRL2, 0);
> + if (ret) {
> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> + MAX9611_REG_CTRL2, 0);
> + return ret;
> + }
> + usleep_range(1000, 2000);
> +
> + /* Make sure die temperature is in range to test communications. */
> + ret = i2c_smbus_read_word_swapped(max9611->i2c_client,
> + MAX9611_REG_TEMP_DATA);
> + if (ret < 0) {
> + dev_err(max9611->dev, "i2c read word from 0x%2x failed\n",
> + MAX9611_REG_TEMP_DATA);
> + return ret;
> + }
> + regval = ret & ~TEMP_MASK;
> +
> + if ((regval > TEMP_MAX_RAW_POS &&
> + regval < TEMP_MIN_RAW_NEG) ||
> + regval > TEMP_MAX_RAW_NEG) {
> + dev_err(max9611->dev,
> + "Invalid value received from ADC 0x%4x: aborting\n",
> + regval);
> + return -EIO;
> + }
> +
> + /* Mux shall be zeroed back before applying other configurations */
> + ret = i2c_smbus_write_byte_data(max9611->i2c_client,
> + MAX9611_REG_CTRL1, 0);
> + if (ret) {
> + dev_err(max9611->dev, "i2c write byte failed: 0x%2x - 0x%2x\n",
> + MAX9611_REG_CTRL1, 0);
> + return ret;
> + }
> + usleep_range(1000, 2000);
> +
> + return 0;
> +}
> +
> +static int max9611_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + const char * const shunt_res_prop = "shunt-resistor-uohm";
> + struct device_node *of_node = client->dev.of_node;
> + struct max9611_dev *max9611;
> + struct iio_dev *indio_dev;
> + unsigned int of_shunt;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*max9611));
> + if (IS_ERR(indio_dev))
> + return PTR_ERR(indio_dev);
> +
> + i2c_set_clientdata(client, indio_dev);
> +
> + max9611 = iio_priv(indio_dev);
> + max9611->dev = &client->dev;
> + max9611->i2c_client = client;
> + mutex_init(&max9611->lock);
> +
> + ret = of_property_read_u32(of_node, shunt_res_prop, &of_shunt);
> + if (ret) {
> + dev_err(&client->dev,
> + "Missing %s property for %s node\n",
> + shunt_res_prop, of_node->full_name);
> + return ret;
> + }
> + max9611->shunt_resistor_uohm = of_shunt;
> +
> + ret = max9611_init(max9611);
> + if (ret)
> + return ret;
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->dev.of_node = client->dev.of_node;
> + indio_dev->name = client->dev.of_node->name;
What's this going to give for the name? Name in IIO is supposed to
be an indication of the part rather than anything more explicit.
That's not easily obtained from device tree directly...
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->info = &indio_info;
> + indio_dev->channels = max9611_channels;
> + indio_dev->num_channels = ARRAY_SIZE(max9611_channels);
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct of_device_id max9611_of_table[] = {
> + {.compatible = "maxim,max9611"},
> + {.compatible = "maxim,max9612"},
> + { },
> +};
> +
> +MODULE_DEVICE_TABLE(of, max9611_of_table);
> +
> +static struct i2c_driver max9611_driver = {
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = max9611_of_table,
> + },
> + .probe = max9611_probe,
> +};
> +module_i2c_driver(max9611_driver);
> +
> +MODULE_AUTHOR("Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
> +MODULE_DESCRIPTION("Maxim max9611/12 current sense amplifier with 12bit ADC");
> +MODULE_LICENSE("GPL v2");
>
--
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 1/5] iio: xoadc: augment DT bindings a bit
From: Jonathan Cameron @ 2017-03-25 15:47 UTC (permalink / raw)
To: Linus Walleij, Rob Herring
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Bjorn Andersson,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CACRpkdaZmauodn+xvSudUELCRrO4j7Dj+SM+H4bkkqQMeJ6EoA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 24/03/17 15:25, Linus Walleij wrote:
> On Fri, Mar 24, 2017 at 4:12 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Sat, Mar 18, 2017 at 02:33:58PM +0100, Linus Walleij wrote:
>
>>> -- #address-cells: should be set to <1>
>>> +- #address-cells: should be set to <2>, the first cell is the
>>> + prescaler (on PM8058) or premux (on PM8921) with two valid bits
>>> + so legal values are 0x00, 0x01 or 0x02. The second cell
>>> + is the main analog mux setting (0x00..0x0f). The combination
>>> + of prescaler/premux and analog mux uniquely addresses a hardware
>>> + channel on all systems.
>>
>> 64-bits to describe 48 possible combinations... I'd just combine these
>> into 1 cell.
>
> That was what I did first but it becomes very unintuitive and
> require translation using tables etc from the datasheets. You
> have to shuffle and stuff bits to get that one cell value.
>
> The addressing is really done in two steps:
>
> premux -> mux -> channel
>
> Example:
>
> Premux 0x02
> Mux 0x08
>
> I guess I could of course put that into one cell using 4+4 bits
> and form 0x28.
>
> On the other hand, that is not how the hardware works, because
> there premux/prescaler and analog mux are two separate things.
>
>>> - reg: should contain the hardware channel number in the range
>>> - 0 .. 0x0f (4 bits). The hardware only supports 16 channels.
>>> + 0 .. 0xff (8 bits).
>>> +
>>> + On PM8058 the hardware only supports 16 channels, but we get the same
>>> + channels repeating with its input divided down by 1 or 3. Channels 00,
>>> + 10, 20, ... f0 are the raw values, 04, 14, 24 .. f4 are "unity" channels
>>> + divided by 1, and 08, 18, 28 .. f8 are channels divided by 3. Bits 0
>>> + and 1 of the channel index should always be 0.
>>> +
>>> + On PM8921 the hardware supports more than 16 channels through a complex
>>> + routing matrix using a premux, so 00, 10, 20 .. f0 are the basic raw
>>> + channels while another set of channels appear for 04, 14, 24 .. f4,
>>> + and again some of the same channels appear again divided down by 3
>>> + in 08, 18, 28 .. f8. Again bits 0 and 1 of the channel index should
>>> + always be 0.
>>
>> Now I'm lost...
>
> That is actually how it looks with the old scheme, which you are
> kind of requesting that I use instead. In a way it's good that I left
> the old documentation in there because it illustrates my problem
> with what you request above: lots of "holes" in that address
> space and very unituitive numbers.
>
I definitely agree that clarity is probably better than worrying about the
few extra bytes...
Jonathan
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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 v3 05/11] IIO: ADC: add stm32 DFSDM support for Sigma delta ADC
From: Jonathan Cameron @ 2017-03-25 15:53 UTC (permalink / raw)
To: Arnaud Pouliquen, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Olivier MOYSAN, kernel@stlinux.com, linux-iio@vger.kernel.org,
Maxime Coquelin, linux-arm-kernel@lists.infradead.org,
Alexandre TORGUE
In-Reply-To: <df0de3f2-1aba-d9d9-4893-255d0c9480e1@st.com>
On 20/03/17 11:24, Arnaud Pouliquen wrote:
> Hello Jonathan
>
> Thanks for your comments
> Few answers in-line.
>
> Regards
> Arnaud
>
> On 03/19/2017 11:25 PM, Jonathan Cameron wrote:
>> On 17/03/17 14:08, Arnaud Pouliquen wrote:
>>> Add driver for stm32 DFSDM IP. This IP converts a sigma delta stream
>>> in n bit samples through a low pass filter and an integrator.
>>> stm32-dfsdm-adc driver allows to handle sigma delta ADC.
>>>
>>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
>> Various minor bits inline.
>>
>> I'm mostly liking this. I do slightly wondering if semantically it
>> should be the front end that has the channels rather than the
>> backend. Would be fiddly to do though and probably not worth the
>> hassle.
> DFSDM support the scan mode, so several front ends can be connected to
> One filter. In this case not possible to expose channel FE.
It still could but would admittedly get really fiddly and require demuxing
the scan...
>
>>
>> Would love to see it running in a continuous mode in IIO, but
>> I guess that can follow along later.
> Yes for the rest of the management it should be quite close to the
> stm32-adc driver.
>
>>
>> The comment about the trigger has me confused
>> - perhaps you could elaborate further on that?
> Code associated to the trigger should be part of the [PATCH v3 06/11]
> IIO: ADC: add stm32 DFSDM support for PDM microphone, as it concern the
> audio part...
> I did not found a way to use consumer.h interface to enable DFSDM IIO,
> without defining triggered buffer. that's why i defined a trigger and
> use it.
> But i just saw that my reasoning is wrong. I'm linked to trigger in
> stm32-dfsdm-audio.c because i use iio_triggered_buffer_postenable and
> iio_triggered_buffer_predisable.
This used to be more obvious until we put those boiler plate functions in
to avoid lots of replication. Pretty much everything should be optional.
> As i don't use the callback for buffer
> no need to call it...i can call the ASoC callback directly in DMA IRQ.
> Still a hack but more logic...
Cool. We definitely need to clean this up long term, but perhaps not
as part of this initially at least!
I hate holding drivers up for internal stuff that we can change in
our own good time!
>
>>
>> Jonathan
>>> ---
>>> V2 -> V3 :
>>> - Split audio and ADC support in 2 drivers
>>> - Implement DMA cyclic mode
>>> - Add SPI bus Trigger for buffer management
>>>
>>> drivers/iio/adc/Kconfig | 26 ++
>>> drivers/iio/adc/Makefile | 2 +
>>> drivers/iio/adc/stm32-dfsdm-adc.c | 419 +++++++++++++++++++++++
>>> drivers/iio/adc/stm32-dfsdm-core.c | 658 +++++++++++++++++++++++++++++++++++++
>>> drivers/iio/adc/stm32-dfsdm.h | 372 +++++++++++++++++++++
>>> 5 files changed, 1477 insertions(+)
>>> create mode 100644 drivers/iio/adc/stm32-dfsdm-adc.c
>>> create mode 100644 drivers/iio/adc/stm32-dfsdm-core.c
>>> create mode 100644 drivers/iio/adc/stm32-dfsdm.h
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index d411d66..3e0eb11 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -452,6 +452,32 @@ config STM32_ADC
>>> This driver can also be built as a module. If so, the module
>>> will be called stm32-adc.
>>>
>>> +config STM32_DFSDM_CORE
>>> + tristate "STMicroelectronics STM32 dfsdm core"
>>> + depends on (ARCH_STM32 && OF) || COMPILE_TEST
>>> + select REGMAP
>>> + select REGMAP_MMIO
>>> + help
>>> + Select this option to enable the driver for STMicroelectronics
>>> + STM32 digital filter for sigma delta converter.
>>> +
>>> + This driver can also be built as a module. If so, the module
>>> + will be called stm32-dfsdm-core.
>>> +
>>> +config STM32_DFSDM_ADC
>>> + tristate "STMicroelectronics STM32 dfsdm adc"
>>> + depends on (ARCH_STM32 && OF) || COMPILE_TEST
>>> + select STM32_DFSDM_CORE
>>> + select REGMAP_MMIO
>>> + select IIO_BUFFER_DMAENGINE
>>> + select IIO_HW_CONSUMER
>>> + help
>>> + Select this option to support ADCSigma delta modulator for
>>> + STMicroelectronics STM32 digital filter for sigma delta converter.
>>> +
>>> + This driver can also be built as a module. If so, the module
>>> + will be called stm32-dfsdm-adc.
>>> +
>>> config STX104
>>> tristate "Apex Embedded Systems STX104 driver"
>>> depends on X86 && ISA_BUS_API
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index c68819c..161f271 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -43,6 +43,8 @@ obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>>> obj-$(CONFIG_STX104) += stx104.o
>>> obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
>>> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>> +obj-$(CONFIG_STM32_DFSDM_ADC) += stm32-dfsdm-adc.o
>>> +obj-$(CONFIG_STM32_DFSDM_CORE) += stm32-dfsdm-core.o
>>> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>> obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
>>> diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
>>> new file mode 100644
>>> index 0000000..ebcb3b4
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32-dfsdm-adc.c
>>> @@ -0,0 +1,419 @@
>>> +/*
>>> + * This file is the ADC part of of the STM32 DFSDM driver
>>> + *
>>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>>> + * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * 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.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/interrupt.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#include <linux/iio/hw_consumer.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +
>>> +#include "stm32-dfsdm.h"
>>> +
>>> +#define DFSDM_TIMEOUT_US 100000
>>> +#define DFSDM_TIMEOUT (msecs_to_jiffies(DFSDM_TIMEOUT_US / 1000))
>>> +
>>> +struct stm32_dfsdm_adc {
>>> + struct stm32_dfsdm *dfsdm;
>>> + unsigned int fl_id;
>>> + unsigned int ch_id;
>>> +
>>> + unsigned int oversamp;
>>> +
>>> + struct completion completion;
>>> +
>>> + u32 *buffer;
>>> +
>>> + /* Hardware consumer structure for Front End IIO */
>>> + struct iio_hw_consumer *hwc;
>>> +};
>>> +
>>> +static int stm32_dfsdm_start_conv(struct stm32_dfsdm_adc *adc)
>>> +{
>>> + int ret;
>>> +
>>> + ret = stm32_dfsdm_start_dfsdm(adc->dfsdm);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = stm32_dfsdm_start_channel(adc->dfsdm, adc->ch_id);
>>> + if (ret < 0)
>>> + goto stop_dfsdm;
>>> +
>>> + ret = stm32_dfsdm_filter_configure(adc->dfsdm, adc->fl_id, adc->ch_id);
>>> + if (ret < 0)
>>> + goto stop_channels;
>>> +
>>> + ret = stm32_dfsdm_start_filter(adc->dfsdm, adc->fl_id);
>>> + if (ret < 0)
>>> + goto stop_channels;
>>> +
>>> + return 0;
>>> +
>>> +stop_channels:
>>> + stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
>>> +stop_dfsdm:
>>> + stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_adc *adc)
>>> +{
>>> + stm32_dfsdm_stop_filter(adc->dfsdm, adc->fl_id);
>>> +
>>> + stm32_dfsdm_stop_channel(adc->dfsdm, adc->ch_id);
>>> +
>>> + stm32_dfsdm_stop_dfsdm(adc->dfsdm);
>>> +}
>>> +
>>> +static int stm32_dfsdm_single_conv(struct iio_dev *indio_dev,
>>> + const struct iio_chan_spec *chan, int *res)
>>> +{
>>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>>> + long timeout;
>>> + int ret;
>>> +
>>> + reinit_completion(&adc->completion);
>>> +
>>> + adc->buffer = res;
>>> +
>>> + /* Unmask IRQ for regular conversion achievement*/
>>> + ret = regmap_update_bits(adc->dfsdm->regmap, DFSDM_CR2(adc->fl_id),
>>> + DFSDM_CR2_REOCIE_MASK, DFSDM_CR2_REOCIE(1));
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = stm32_dfsdm_start_conv(adc);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + timeout = wait_for_completion_interruptible_timeout(&adc->completion,
>>> + DFSDM_TIMEOUT);
>> blank line perhaps.
>>> + /* Mask IRQ for regular conversion achievement*/
>>> + regmap_update_bits(adc->dfsdm->regmap, DFSDM_CR2(adc->fl_id),
>>> + DFSDM_CR2_REOCIE_MASK, DFSDM_CR2_REOCIE(0));
>>> +
>>> + if (timeout == 0) {
>>> + dev_warn(&indio_dev->dev, "Conversion timed out!\n");
>>> + ret = -ETIMEDOUT;
>>> + } else if (timeout < 0) {
>>> + ret = timeout;
>>> + } else {
>>> + dev_dbg(&indio_dev->dev, "Converted val %#x\n", *res);
>>> + ret = IIO_VAL_INT;
>>> + }
>>> +
>>> + /* Mask IRQ for regular conversion achievement*/
>>> + regmap_update_bits(adc->dfsdm->regmap, DFSDM_CR2(adc->fl_id),
>>> + DFSDM_CR2_REOCIE_MASK, DFSDM_CR2_REOCIE(0));
>>> +
>>> + stm32_dfsdm_stop_conv(adc);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int stm32_dfsdm_write_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan,
>>> + int val, int val2, long mask)
>>> +{
>>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>>> + struct stm32_dfsdm_filter *fl = &adc->dfsdm->fl_list[adc->fl_id];
>>> + int ret = -EINVAL;
>>> +
>>> + if (mask == IIO_CHAN_INFO_OVERSAMPLING_RATIO) {
>>> + ret = stm32_dfsdm_set_osrs(fl, 0, val);
>>> + if (!ret)
>>> + adc->oversamp = val;
>>> + }
>> blank line here.
>>> + return ret;
>>> +}
>>> +
>>> +static int stm32_dfsdm_read_raw(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec const *chan, int *val,
>>> + int *val2, long mask)
>>> +{
>>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + switch (mask) {
>>> + case IIO_CHAN_INFO_RAW:
>>> + ret = iio_hw_consumer_enable(adc->hwc);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + "%s: IIO enable failed (channel %d)\n",
>>> + __func__, chan->channel);
>>> + return ret;
>>> + }
>>> + ret = stm32_dfsdm_single_conv(indio_dev, chan, val);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + "%s: Conversion failed (channel %d)\n",
>>> + __func__, chan->channel);
>>> + return ret;
>>> + }
>>> +
>>> + iio_hw_consumer_disable(adc->hwc);
>>> +
>>> + return IIO_VAL_INT;
>>> +
>>> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>>> + *val = adc->oversamp;
>>> +
>>> + return IIO_VAL_INT;
>>> + }
>>> +
>>> + return -EINVAL;
>>> +}
>>> +
>>> +static const struct iio_info stm32_dfsdm_info_adc = {
>>> + .read_raw = stm32_dfsdm_read_raw,
>>> + .write_raw = stm32_dfsdm_write_raw,
>>> + .driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static irqreturn_t stm32_dfsdm_irq(int irq, void *arg)
>>> +{
>>> + struct stm32_dfsdm_adc *adc = arg;
>>> + struct regmap *regmap = adc->dfsdm->regmap;
>>> + unsigned int status;
>>> +
>>> + regmap_read(regmap, DFSDM_ISR(adc->fl_id), &status);
>>> +
>>> + if (status & DFSDM_ISR_REOCF_MASK) {
>>> + /* read the data register clean the IRQ status */
>>> + regmap_read(regmap, DFSDM_RDATAR(adc->fl_id), adc->buffer);
>>> + complete(&adc->completion);
>>> + }
>>> + if (status & DFSDM_ISR_ROVRF_MASK) {
>> What's this one? Might want a comment given it's an irq you basically eat.
> Yes at least an error message that to inform on an overrun.
>>> + regmap_update_bits(regmap, DFSDM_ICR(adc->fl_id),
>>> + DFSDM_ICR_CLRROVRF_MASK,
>>> + DFSDM_ICR_CLRROVRF_MASK);
>>> + }
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>>> +
>>> + return stm32_dfsdm_start_conv(adc);
>>> +}
>>> +
>>> +static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>>> +
>>> + stm32_dfsdm_stop_conv(adc);
>> blank line.
>>> + return 0;
>>> +}
>>> +
>>> +static const struct iio_buffer_setup_ops stm32_dfsdm_buffer_setup_ops = {
>>> + .postenable = &stm32_dfsdm_postenable,
>>> + .predisable = &stm32_dfsdm_predisable,
>>> +};
>>> +
>>> +static int stm32_dfsdm_adc_chan_init_one(struct iio_dev *indio_dev,
>>> + struct iio_chan_spec *chan,
>>> + int ch_idx)
>>> +{
>>> + struct iio_chan_spec *ch = &chan[ch_idx];
>>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + ret = stm32_dfsdm_channel_parse_of(adc->dfsdm, indio_dev, chan, ch_idx);
>>> +
>>> + ch->type = IIO_VOLTAGE;
>>> + ch->indexed = 1;
>>> + ch->scan_index = ch_idx;
>>> +
>>> + /*
>>> + * IIO_CHAN_INFO_RAW: used to compute regular conversion
>>> + * IIO_CHAN_INFO_OVERSAMPLING_RATIO: used to set oversampling
>>> + */
>>> + ch->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO);
>>> +
>>> + ch->scan_type.sign = 'u';
>>> + ch->scan_type.realbits = 24;
>>> + ch->scan_type.storagebits = 32;
>>> + adc->ch_id = ch->channel;
>>> +
>>> + return stm32_dfsdm_chan_configure(adc->dfsdm,
>>> + &adc->dfsdm->ch_list[ch->channel]);
>>> +}
>>> +
>>> +static int stm32_dfsdm_adc_chan_init(struct iio_dev *indio_dev)
>>> +{
>>> + struct iio_chan_spec *channels;
>>> + struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
>>> + unsigned int num_ch;
>>> + int ret, chan_idx;
>>> +
>>> + num_ch = of_property_count_u32_elems(indio_dev->dev.of_node,
>>> + "st,adc-channels");
>>> + if (num_ch < 0 || num_ch >= adc->dfsdm->num_chs) {
>>> + dev_err(&indio_dev->dev, "Bad st,adc-channels?\n");
>>> + return num_ch < 0 ? num_ch : -EINVAL;
>>> + }
>>> +
>>> + /*
>>> + * Number of channel per filter is temporary limited to 1.
>>> + * Restriction should be cleaned with scan mode
>>> + */
>>> + if (num_ch > 1) {
>>> + dev_err(&indio_dev->dev, "Multi channel not yet supported\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* Bind to SD modulator IIO device */
>>> + adc->hwc = iio_hw_consumer_alloc(&indio_dev->dev);
>>> + if (IS_ERR(adc->hwc))
>>> + return -EPROBE_DEFER;
>>> +
>>> + channels = devm_kcalloc(&indio_dev->dev, num_ch, sizeof(*channels),
>>> + GFP_KERNEL);
>>> + if (!channels)
>>> + return -ENOMEM;
>>> +
>>> + for (chan_idx = 0; chan_idx < num_ch; chan_idx++) {
>>> + ret = stm32_dfsdm_adc_chan_init_one(indio_dev, channels,
>>> + chan_idx);
>>> + if (ret < 0)
>>> + goto free_hwc;
>>> + }
>>> +
>>> + indio_dev->num_channels = num_ch;
>>> + indio_dev->channels = channels;
>>> +
>>> + return 0;
>>> +
>>> +free_hwc:
>>> + iio_hw_consumer_free(adc->hwc);
>> Given you have to free this in the error path, I would imagine you will
>> need a free somewhere in the main remove path? Or just create a devm
>> version of iio_hw_consumer_alloc. It will be useful in the long run.
>>> + return ret;
>>> +}
>>> +
>>> +static const struct of_device_id stm32_dfsdm_adc_match[] = {
>>> + { .compatible = "st,stm32-dfsdm-adc"},
>>> + {}
>>> +};
>>> +
>>> +static int stm32_dfsdm_adc_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct stm32_dfsdm_adc *adc;
>>> + struct device_node *np = dev->of_node;
>>> + struct iio_dev *iio;
>>> + char *name;
>>> + int ret, irq, val;
>>> +
>>> + iio = devm_iio_device_alloc(dev, sizeof(*adc));
>>> + if (IS_ERR(iio)) {
>>> + dev_err(dev, "%s: Failed to allocate IIO\n", __func__);
>>> + return PTR_ERR(iio);
>>> + }
>>> +
>>> + adc = iio_priv(iio);
>>> + if (IS_ERR(adc)) {
>>> + dev_err(dev, "%s: Failed to allocate ADC\n", __func__);
>>> + return PTR_ERR(adc);
>>> + }
>>> + adc->dfsdm = dev_get_drvdata(dev->parent);
>>> +
>>> + iio->dev.parent = dev;
>>> + iio->dev.of_node = np;
>>> + iio->info = &stm32_dfsdm_info_adc;
>>> + iio->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
>>> +
>>> + platform_set_drvdata(pdev, adc);
>>> +
>>> + ret = of_property_read_u32(dev->of_node, "reg", &adc->fl_id);
>>> + if (ret != 0) {
>>> + dev_err(dev, "Missing reg property\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + name = kzalloc(sizeof("dfsdm-adc0"), GFP_KERNEL);
>> not freed. Maybe devm_kzalloc
>>> + if (!name)
>>> + return -ENOMEM;
>>> + snprintf(name, sizeof("dfsdm-adc0"), "dfsdm-adc%d", adc->fl_id);
>>> + iio->name = name;
>>> +
>>> + /*
>>> + * In a first step IRQs generated for channels are not treated.
>>> + * So IRQ associated to filter instance 0 is dedicated to the Filter 0.
>>> + */
>>> + irq = platform_get_irq(pdev, 0);
>>> + ret = devm_request_irq(dev, irq, stm32_dfsdm_irq,
>>> + 0, pdev->name, adc);
>>> + if (ret < 0) {
>>> + dev_err(dev, "Failed to request IRQ\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = of_property_read_u32(dev->of_node, "st,filter-order", &val);
>>> + if (ret < 0) {
>>> + dev_err(dev, "Failed to set filter order\n");
>>> + return ret;
>>> + }
>>> + adc->dfsdm->fl_list[adc->fl_id].ford = val;
>>> +
>>> + ret = of_property_read_u32(dev->of_node, "st,filter0-sync", &val);
>>> + if (!ret)
>>> + adc->dfsdm->fl_list[adc->fl_id].sync_mode = val;
>>> +
>>> + ret = stm32_dfsdm_adc_chan_init(iio);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + init_completion(&adc->completion);
>>> +
>>> + return iio_device_register(iio);
>>> +}
>>> +
>>> +static int stm32_dfsdm_adc_remove(struct platform_device *pdev)
>>> +{
>>> + struct stm32_dfsdm_adc *adc = platform_get_drvdata(pdev);
>>> + struct iio_dev *iio = iio_priv_to_dev(adc);
>>> +
>>> + iio_device_unregister(iio);
>> If all you have is this in remove, you can probably get away with
>> devm_iio_device_register and get rid of the remove entirely.
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct platform_driver stm32_dfsdm_adc_driver = {
>>> + .driver = {
>>> + .name = "stm32-dfsdm-adc",
>>> + .of_match_table = stm32_dfsdm_adc_match,
>>> + },
>>> + .probe = stm32_dfsdm_adc_probe,
>>> + .remove = stm32_dfsdm_adc_remove,
>>> +};
>>> +module_platform_driver(stm32_dfsdm_adc_driver);
>>> +
>>> +MODULE_DESCRIPTION("STM32 sigma delta ADC");
>>> +MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/iio/adc/stm32-dfsdm-core.c b/drivers/iio/adc/stm32-dfsdm-core.c
>>> new file mode 100644
>>> index 0000000..488e456
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32-dfsdm-core.c
>>> @@ -0,0 +1,658 @@
>>> +/*
>>> + * This file is part the core part STM32 DFSDM driver
>>> + *
>>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>>> + * Author(s): Arnaud Pouliquen <arnaud.pouliquen@st.com> for STMicroelectronics.
>>> + *
>>> + * License terms: GPL V2.0.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/sysfs.h>
>>> +
>>> +#include "stm32-dfsdm.h"
>>> +
>>> +struct stm32_dfsdm_dev_data {
>>> + unsigned int num_filters;
>>> + unsigned int num_channels;
>>> + const struct regmap_config *regmap_cfg;
>>> +};
>>> +
>>> +#define STM32H7_DFSDM_NUM_FILTERS 4
>>> +#define STM32H7_DFSDM_NUM_CHANNELS 8
>>> +
>>> +#define DFSDM_MAX_INT_OVERSAMPLING 256
>>> +
>>> +#define DFSDM_MAX_FL_OVERSAMPLING 1024
>>> +
>>> +#define DFSDM_MAX_RES BIT(31)
>>> +#define DFSDM_DATA_RES BIT(23)
>>> +
>>> +static bool stm32_dfsdm_volatile_reg(struct device *dev, unsigned int reg)
>>> +{
>>> + if (reg < DFSDM_FILTER_BASE_ADR)
>>> + return false;
>>> +
>>> + /*
>>> + * Mask is done on register to avoid to list registers of all them
>>> + * filter instances.
>>> + */
>>> + switch (reg & DFSDM_FILTER_REG_MASK) {
>>> + case DFSDM_CR1(0) & DFSDM_FILTER_REG_MASK:
>>> + case DFSDM_ISR(0) & DFSDM_FILTER_REG_MASK:
>>> + case DFSDM_JDATAR(0) & DFSDM_FILTER_REG_MASK:
>>> + case DFSDM_RDATAR(0) & DFSDM_FILTER_REG_MASK:
>>> + return true;
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static const struct regmap_config stm32h7_dfsdm_regmap_cfg = {
>>> + .reg_bits = 32,
>>> + .val_bits = 32,
>>> + .reg_stride = sizeof(u32),
>>> + .max_register = 0x2B8,
>>> + .volatile_reg = stm32_dfsdm_volatile_reg,
>>> + .fast_io = true,
>>> +};
>>> +
>>> +static const struct stm32_dfsdm_dev_data stm32h7_dfsdm_data = {
>>> + .num_filters = STM32H7_DFSDM_NUM_FILTERS,
>>> + .num_channels = STM32H7_DFSDM_NUM_CHANNELS,
>>> + .regmap_cfg = &stm32h7_dfsdm_regmap_cfg,
>>> +};
>>> +
>>> +struct dfsdm_priv {
>>> + struct platform_device *pdev; /* platform device*/
>>> +
>>> + struct stm32_dfsdm dfsdm; /* common data exported for all instances */
>>> +
>>> + unsigned int spi_clk_out_div; /* SPI clkout divider value */
>>> + atomic_t n_active_ch; /* number of current active channels */
>>> +
>>> + /* Clock */
>>> + struct clk *clk; /* DFSDM clock */
>>> + struct clk *aclk; /* audio clock */
>>> +};
>>> +
>>> +/**
>>> + * stm32_dfsdm_set_osrs - compute filter parameters.
>> Naming would suggest it's more specific than this.
>> Setting over sampling ratios?
> Right, it is a computation not a set.
>>> + *
>>> + * Enable interface if n_active_ch is not null.
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + * @fast: Fast mode enabled or disabled
>>> + * @oversamp: Expected oversampling between filtered sample and SD input stream
>>> + */
>>> +int stm32_dfsdm_set_osrs(struct stm32_dfsdm_filter *fl, unsigned int fast,
>>> + unsigned int oversamp)
>>> +{
>>> + unsigned int i, d, fosr, iosr;
>>> + u64 res;
>>> + s64 delta;
>>> + unsigned int m = 1; /* multiplication factor */
>>> + unsigned int p = fl->ford; /* filter order (ford) */
>>> +
>>> + pr_debug("%s: Requested oversampling: %d\n", __func__, oversamp);
>>> + /*
>>> + * This function tries to compute filter oversampling and integrator
>>> + * oversampling, base on oversampling ratio requested by user.
>>> + *
>>> + * Decimation d depends on the filter order and the oversampling ratios.
>>> + * ford: filter order
>>> + * fosr: filter over sampling ratio
>>> + * iosr: integrator over sampling ratio
>>> + */
>>> + if (fl->ford == DFSDM_FASTSINC_ORDER) {
>>> + m = 2;
>>> + p = 2;
>>> + }
>>> +
>>> + /*
>>> + * Looks for filter and integrator oversampling ratios which allows
>>> + * to reach 24 bits data output resolution.
>>> + * Leave at once if exact resolution if reached.
>>> + * Otherwise the higher resolution below 32 bits is kept.
>>> + */
>>> + for (fosr = 1; fosr <= DFSDM_MAX_FL_OVERSAMPLING; fosr++) {
>>> + for (iosr = 1; iosr <= DFSDM_MAX_INT_OVERSAMPLING; iosr++) {
>>> + if (fast)
>>> + d = fosr * iosr;
>>> + else if (fl->ford == DFSDM_FASTSINC_ORDER)
>>> + d = fosr * (iosr + 3) + 2;
>>> + else
>>> + d = fosr * (iosr - 1 + p) + p;
>>> +
>>> + if (d > oversamp)
>>> + break;
>>> + else if (d != oversamp)
>>> + continue;
>>> + /*
>>> + * Check resolution (limited to signed 32 bits)
>>> + * res <= 2^31
>>> + * Sincx filters:
>>> + * res = m * fosr^p x iosr (with m=1, p=ford)
>>> + * FastSinc filter
>>> + * res = m * fosr^p x iosr (with m=2, p=2)
>>> + */
>>> + res = fosr;
>>> + for (i = p - 1; i > 0; i--) {
>>> + res = res * (u64)fosr;
>>> + if (res > DFSDM_MAX_RES)
>>> + break;
>>> + }
>>> + if (res > DFSDM_MAX_RES)
>>> + continue;
>>> + res = res * (u64)m * (u64)iosr;
>>> + if (res > DFSDM_MAX_RES)
>>> + continue;
>>> +
>>> + delta = res - DFSDM_DATA_RES;
>>> +
>>> + if (res >= fl->res) {
>>> + fl->res = res;
>>> + fl->fosr = fosr;
>>> + fl->iosr = iosr;
>>> + fl->fast = fast;
>>> + pr_debug("%s: fosr = %d, iosr = %d\n",
>>> + __func__, fl->fosr, fl->iosr);
>>> + }
>>> +
>>> + if (!delta)
>>> + return 0;
>>> + }
>>> + }
>>> +
>>> + if (!fl->fosr)
>>> + return -EINVAL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_start_dfsdm - start global dfsdm IP interface.
>>> + *
>>> + * Enable interface if n_active_ch is not null.
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + */
>>> +int stm32_dfsdm_start_dfsdm(struct stm32_dfsdm *dfsdm)
>>> +{
>>> + struct dfsdm_priv *priv = container_of(dfsdm, struct dfsdm_priv, dfsdm);
>>> + struct device *dev = &priv->pdev->dev;
>>> + unsigned int clk_div = priv->spi_clk_out_div;
>>> + int ret;
>>> +
>>> + if (atomic_inc_return(&priv->n_active_ch) == 1) {
>>> + /* Enable clocks */
>>> + ret = clk_prepare_enable(priv->clk);
>>> + if (ret < 0) {
>>> + dev_err(dev, "Failed to start clock\n");
>>> + return ret;
>>> + }
>>> + if (priv->aclk) {
>>> + ret = clk_prepare_enable(priv->aclk);
>>> + if (ret < 0) {
>>> + dev_err(dev, "Failed to start audio clock\n");
>>> + goto disable_clk;
>>> + }
>>> + }
>>> +
>>> + /* Output the SPI CLKOUT (if clk_div == 0 clock if OFF) */
>>> + ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
>>> + DFSDM_CHCFGR1_CKOUTDIV_MASK,
>>> + DFSDM_CHCFGR1_CKOUTDIV(clk_div));
>>> + if (ret < 0)
>>> + goto disable_aclk;
>>> +
>>> + /* Global enable of DFSDM interface */
>>> + ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
>>> + DFSDM_CHCFGR1_DFSDMEN_MASK,
>>> + DFSDM_CHCFGR1_DFSDMEN(1));
>>> + if (ret < 0)
>>> + goto disable_aclk;
>>> + }
>>> +
>>> + dev_dbg(dev, "%s: n_active_ch %d\n", __func__,
>>> + atomic_read(&priv->n_active_ch));
>>> +
>>> + return 0;
>>> +
>>> +disable_aclk:
>>> + clk_disable_unprepare(priv->aclk);
>>> +disable_clk:
>>> + clk_disable_unprepare(priv->clk);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_stop_dfsdm - stop global DFSDM IP interface.
>>> + *
>>> + * Disable interface if n_active_ch is null
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + */
>>> +int stm32_dfsdm_stop_dfsdm(struct stm32_dfsdm *dfsdm)
>>> +{
>>> + struct dfsdm_priv *priv = container_of(dfsdm, struct dfsdm_priv, dfsdm);
>>> + int ret;
>>> +
>>> + if (atomic_dec_and_test(&priv->n_active_ch)) {
>>> + /* Global disable of DFSDM interface */
>>> + ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
>>> + DFSDM_CHCFGR1_DFSDMEN_MASK,
>>> + DFSDM_CHCFGR1_DFSDMEN(0));
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Stop SPI CLKOUT */
>>> + ret = regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(0),
>>> + DFSDM_CHCFGR1_CKOUTDIV_MASK,
>>> + DFSDM_CHCFGR1_CKOUTDIV(0));
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Disable clocks */
>>> + clk_disable_unprepare(priv->clk);
>>> + if (priv->aclk)
>>> + clk_disable_unprepare(priv->aclk);
>>> + }
>>> + dev_dbg(&priv->pdev->dev, "%s: n_active_ch %d\n", __func__,
>>> + atomic_read(&priv->n_active_ch));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_start_channel
>>> + * Start DFSDM IP channels and associated interface.
>>> + *
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + * @ch_id: Channel index.
>>> + */
>>> +int stm32_dfsdm_start_channel(struct stm32_dfsdm *dfsdm, unsigned int ch_id)
>>> +{
>>> + return regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(ch_id),
>>> + DFSDM_CHCFGR1_CHEN_MASK,
>>> + DFSDM_CHCFGR1_CHEN(1));
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_stop_channel
>>> + * Stop DFSDM IP channels and associated interface.
>>> + *
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + * @ch_id: Channel index.
>>> + */
>>> +void stm32_dfsdm_stop_channel(struct stm32_dfsdm *dfsdm, unsigned int ch_id)
>>> +{
>>> + regmap_update_bits(dfsdm->regmap, DFSDM_CHCFGR1(ch_id),
>>> + DFSDM_CHCFGR1_CHEN_MASK,
>>> + DFSDM_CHCFGR1_CHEN(0));
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_chan_configure
>>> + * Configure DFSDM IP channels and associated interface.
>>> + *
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + * @ch_id: channel index.
>>> + */
>>> +int stm32_dfsdm_chan_configure(struct stm32_dfsdm *dfsdm,
>>> + struct stm32_dfsdm_channel *ch)
>>> +{
>>> + unsigned int id = ch->id;
>>> + struct regmap *regmap = dfsdm->regmap;
>>> + int ret;
>>> +
>>> + ret = regmap_update_bits(regmap, DFSDM_CHCFGR1(id),
>>> + DFSDM_CHCFGR1_SITP_MASK,
>>> + DFSDM_CHCFGR1_SITP(ch->type));
>>> + if (ret < 0)
>>> + return ret;
>> Blank line here and in similar places makes it easier for my
>> eyes to parse at least...
>> I'd also like to see some docs in here, not all of these
>> are self explanatory.
> I will apply recommendation in my whole code for next time
>>> + ret = regmap_update_bits(regmap, DFSDM_CHCFGR1(id),
>>> + DFSDM_CHCFGR1_SPICKSEL_MASK,
>>> + DFSDM_CHCFGR1_SPICKSEL(ch->src));
>>> + if (ret < 0)
>>> + return ret;
>>> + return regmap_update_bits(regmap, DFSDM_CHCFGR1(id),
>>> + DFSDM_CHCFGR1_CHINSEL_MASK,
>>> + DFSDM_CHCFGR1_CHINSEL(ch->alt_si));
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_start_filter - Start DFSDM IP filter conversion.
>>> + *
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + * @fl_id: Filter index.
>>> + */
>>> +int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
>>> +{
>>> + int ret;
>>> +
>>> + /* Enable filter */
>>> + ret = regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
>>> + DFSDM_CR1_DFEN_MASK, DFSDM_CR1_DFEN(1));
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Start conversion */
>>> + return regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
>>> + DFSDM_CR1_RSWSTART_MASK,
>>> + DFSDM_CR1_RSWSTART(1));
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_stop_filter - Stop DFSDM IP filter conversion.
>>> + *
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + * @fl_id: Filter index.
>>> + */
>>> +void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id)
>>> +{
>>> + /* Mask IRQ for regular conversion achievement*/
>>> + regmap_update_bits(dfsdm->regmap, DFSDM_CR2(fl_id),
>>> + DFSDM_CR2_REOCIE_MASK, DFSDM_CR2_REOCIE(0));
>>> + /* Disable conversion */
>>> + regmap_update_bits(dfsdm->regmap, DFSDM_CR1(fl_id),
>>> + DFSDM_CR1_DFEN_MASK, DFSDM_CR1_DFEN(0));
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_filter_configure - Configure DFSDM IP filter and associate it
>>> + * to channel.
>>> + *
>>> + * @dfsdm: Handle used to retrieve dfsdm context.
>>> + * @fl_id: channel index.
>>> + * @fl_id: Filter index.
>>> + */
>>> +int stm32_dfsdm_filter_configure(struct stm32_dfsdm *dfsdm, unsigned int fl_id,
>>> + unsigned int ch_id)
>>> +{
>>> + struct regmap *regmap = dfsdm->regmap;
>>> + struct stm32_dfsdm_filter *fl = &dfsdm->fl_list[fl_id];
>>> + int ret;
>>> +
>>> + /* Average integrator oversampling */
>>> + ret = regmap_update_bits(regmap, DFSDM_FCR(fl_id), DFSDM_FCR_IOSR_MASK,
>>> + DFSDM_FCR_IOSR(fl->iosr));
>>> +
>>> + /* Filter order and Oversampling */
>> Please handle each error properly as it happens rather than mudling onwards.
>> If there is reason for this odd construction, then document it clearly.
> If you mention the checks on ret value that are missing at end of
> functions, yes dirty code to fix.
>
>>> + if (!ret)
>>> + ret = regmap_update_bits(regmap, DFSDM_FCR(fl_id),
>>> + DFSDM_FCR_FOSR_MASK,
>>> + DFSDM_FCR_FOSR(fl->fosr));
>>> +
>>> + if (!ret)
>>> + ret = regmap_update_bits(regmap, DFSDM_FCR(fl_id),
>>> + DFSDM_FCR_FORD_MASK,
>>> + DFSDM_FCR_FORD(fl->ford));
>>> +
>>> + /* If only one channel no scan mode supported for the moment */
>>> + ret = regmap_update_bits(regmap, DFSDM_CR1(fl_id),
>>> + DFSDM_CR1_RCH_MASK,
>>> + DFSDM_CR1_RCH(ch_id));
>>> +
>>> + return regmap_update_bits(regmap, DFSDM_CR1(fl_id),
>>> + DFSDM_CR1_RSYNC_MASK,
>>> + DFSDM_CR1_RSYNC(fl->sync_mode));
>>> +}
>>> +
>>> +static const struct iio_trigger_ops dfsdm_trigger_ops = {
>>> + .owner = THIS_MODULE,
>>> +};
>>> +
>>> +static int stm32_dfsdm_setup_spi_trigger(struct platform_device *pdev,
>>> + struct stm32_dfsdm *dfsdm)
>>> +{
>>> + /*
>>> + * To be able to use buffer consumer interface a trigger is needed.
>>> + * As conversion are trigged by PDM samples from SPI bus, that makes
>>> + * sense to define the serial interface ( SPI or manchester) as
>>> + * trigger source.
>> It's not actually the case that you have to have a triggrer.
>> There are plenty of drivers (particularly ones with hardware buffering)
>> where there is no trigger envolved. That's not to say it doesn't make sense
>> here.
>>
>> I'm not entirely sure yet if it's needed... Given it has no ops, I doubt it
>> somewhat...
>>> + */
>>> +
>>> + struct iio_trigger *trig;
>>> + int ret;
>>> +
>>> + trig = devm_iio_trigger_alloc(&pdev->dev, DFSDM_SPI_TRIGGER_NAME);
>>> + if (!trig)
>>> + return -ENOMEM;
>>> +
>>> + trig->dev.parent = pdev->dev.parent;
>>> + trig->ops = &dfsdm_trigger_ops;
>>> +
>>> + iio_trigger_set_drvdata(trig, dfsdm);
>>> +
>>> + ret = devm_iio_trigger_register(&pdev->dev, trig);
>>> + if (ret)
>>> + return ret;
>> Just return ret in all cases.
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
>>> + struct iio_dev *indio_dev,
>>> + struct iio_chan_spec *chan, int chan_idx)
>>> +{
>>> + struct iio_chan_spec *ch = &chan[chan_idx];
>>> + struct stm32_dfsdm_channel *df_ch;
>>> + const char *of_str;
>>> + int ret, val;
>>> +
>>> + ret = of_property_read_u32_index(indio_dev->dev.of_node,
>>> + "st,adc-channels", chan_idx,
>>> + &ch->channel);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + " Error parsing 'st,adc-channels' for idx %d\n",
>>> + chan_idx);
>>> + return ret;
>>> + }
>>> +
>>> + ret = of_property_read_string_index(indio_dev->dev.of_node,
>>> + "st,adc-channel-names", chan_idx,
>>> + &ch->datasheet_name);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + " Error parsing 'st,adc-channel-names' for idx %d\n",
>>> + chan_idx);
>>> + return ret;
>>> + }
>>> +
>>> + df_ch = &dfsdm->ch_list[ch->channel];
>> Extra space on the line above.
>>> + df_ch->id = ch->channel;
>>> + ret = of_property_read_string_index(indio_dev->dev.of_node,
>>> + "st,adc-channel-types", chan_idx,
>>> + &of_str);
>>> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_type);
>>> + if (ret < 0 || val < 0)
>>> + df_ch->type = 0;
>>> + else
>>> + df_ch->type = val;
>>> +
>>> + ret = of_property_read_string_index(indio_dev->dev.of_node,
>>> + "st,adc-channel-clk-src", chan_idx,
>>> + &of_str);
>>> + val = stm32_dfsdm_str2val(of_str, stm32_dfsdm_chan_src);
>>> + if (ret < 0 || val < 0)
>>> + df_ch->src = 0;
>>> + else
>>> + df_ch->src = val;
>>> +
>>> + ret = of_property_read_u32_index(indio_dev->dev.of_node,
>>> + "st,adc-alt-channel", chan_idx,
>>> + &df_ch->alt_si);
>>> + if (ret < 0)
>>> + df_ch->alt_si = 0;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_dfsdm_parse_of(struct platform_device *pdev,
>>> + struct dfsdm_priv *priv)
>>> +{
>>> + struct device_node *node = pdev->dev.of_node;
>>> + struct resource *res;
>>> + unsigned long clk_freq;
>>> + unsigned int spi_freq, rem;
>>> + int ret;
>>> +
>>> + if (!node)
>>> + return -EINVAL;
>>> +
>>> + /* Get resources */
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + if (!res) {
>>> + dev_err(&pdev->dev, "Failed to get memory resource\n");
>>> + return -ENODEV;
>>> + }
>>> + priv->dfsdm.phys_base = res->start;
>>> + priv->dfsdm.base = devm_ioremap_resource(&pdev->dev, res);
>>> +
>>> + /* Source clock */
>>> + priv->clk = devm_clk_get(&pdev->dev, "dfsdm");
>>> + if (IS_ERR(priv->clk)) {
>>> + dev_err(&pdev->dev, "No stm32_dfsdm_clk clock found\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + priv->aclk = devm_clk_get(&pdev->dev, "audio");
>>> + if (IS_ERR(priv->aclk))
>>> + priv->aclk = NULL;
>>> +
>>> + if (priv->aclk)
>>> + clk_freq = clk_get_rate(priv->aclk);
>>> + else
>>> + clk_freq = clk_get_rate(priv->clk);
>>> +
>>> + /* SPI clock freq */
>>> + ret = of_property_read_u32(pdev->dev.of_node, "spi-max-frequency",
>>> + &spi_freq);
>>> + if (ret < 0) {
>>> + dev_err(&pdev->dev, "Failed to get spi-max-frequency\n");
>>> + return ret;
>>> + }
>>> +
>>> + priv->spi_clk_out_div = div_u64_rem(clk_freq, spi_freq, &rem) - 1;
>>> + priv->dfsdm.spi_master_freq = spi_freq;
>>> +
>>> + if (rem) {
>>> + dev_warn(&pdev->dev, "SPI clock not accurate\n");
>>> + dev_warn(&pdev->dev, "%ld = %d * %d + %d\n",
>>> + clk_freq, spi_freq, priv->spi_clk_out_div + 1, rem);
>>> + }
>>> +
>>> + return 0;
>>> +};
>>> +
>>> +static const struct of_device_id stm32_dfsdm_of_match[] = {
>>> + {
>>> + .compatible = "st,stm32h7-dfsdm",
>>> + .data = &stm32h7_dfsdm_data,
>>> + },
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(of, stm32_dfsdm_of_match);
>>> +
>>> +static int stm32_dfsdm_remove(struct platform_device *pdev)
>>> +{
>>> + of_platform_depopulate(&pdev->dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_dfsdm_probe(struct platform_device *pdev)
>>> +{
>>> + struct dfsdm_priv *priv;
>>> + struct device_node *pnode = pdev->dev.of_node;
>>> + const struct of_device_id *of_id;
>>> + const struct stm32_dfsdm_dev_data *dev_data;
>>> + struct stm32_dfsdm *dfsdm;
>>> + int ret, i;
>>> +
>>> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>>> + if (!priv)
>>> + return -ENOMEM;
>>> +
>>> + priv->pdev = pdev;
>>> +
>>> + /* Populate data structure depending on compatibility */
>>> + of_id = of_match_node(stm32_dfsdm_of_match, pnode);
>>> + if (!of_id->data) {
>>> + dev_err(&pdev->dev, "Data associated to device is missing\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + dev_data = (const struct stm32_dfsdm_dev_data *)of_id->data;
>>> + dfsdm = &priv->dfsdm;
>>> + dfsdm->fl_list = devm_kcalloc(&pdev->dev, dev_data->num_filters,
>>> + sizeof(*dfsdm->fl_list), GFP_KERNEL);
>>> + if (!dfsdm->fl_list)
>>> + return -ENOMEM;
>>> +
>>> + dfsdm->num_fls = dev_data->num_filters;
>>> + dfsdm->ch_list = devm_kcalloc(&pdev->dev, dev_data->num_channels,
>>> + sizeof(*dfsdm->ch_list),
>>> + GFP_KERNEL);
>>> + if (!dfsdm->ch_list)
>>> + return -ENOMEM;
>>> + dfsdm->num_chs = dev_data->num_channels;
>>> +
>>> + ret = stm32_dfsdm_parse_of(pdev, priv);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + dfsdm->regmap = devm_regmap_init_mmio(&pdev->dev, dfsdm->base,
>>> + &stm32h7_dfsdm_regmap_cfg);
>>> + if (IS_ERR(dfsdm->regmap)) {
>>> + ret = PTR_ERR(dfsdm->regmap);
>>> + dev_err(&pdev->dev, "%s: Failed to allocate regmap: %d\n",
>>> + __func__, ret);
>>> + return ret;
>>> + }
>>> +
>>> + for (i = 0; i < STM32H7_DFSDM_NUM_FILTERS; i++) {
>>> + struct stm32_dfsdm_filter *fl = &dfsdm->fl_list[i];
>>> +
>>> + fl->id = i;
>> I'd like a comment on why this is needed...
> to be cleaned.
>>> + }
>>> +
>>> + platform_set_drvdata(pdev, dfsdm);
>>> +
>>> + ret = stm32_dfsdm_setup_spi_trigger(pdev, dfsdm);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return of_platform_populate(pnode, NULL, NULL, &pdev->dev);
>>> +}
>>> +
>>> +static struct platform_driver stm32_dfsdm_driver = {
>>> + .probe = stm32_dfsdm_probe,
>>> + .remove = stm32_dfsdm_remove,
>>> + .driver = {
>>> + .name = "stm32-dfsdm",
>>> + .of_match_table = stm32_dfsdm_of_match,
>>> + },
>>> +};
>>> +
>>> +module_platform_driver(stm32_dfsdm_driver);
>>> +
>>> +MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 dfsdm driver");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/iio/adc/stm32-dfsdm.h b/drivers/iio/adc/stm32-dfsdm.h
>>> new file mode 100644
>>> index 0000000..bb7d74f
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32-dfsdm.h
>>> @@ -0,0 +1,371 @@
>>> +/*
>>> + * This file is part of STM32 DFSDM driver
>>> + *
>>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>>> + * Author(s): Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>>> + *
>>> + * License terms: GPL V2.0.
>>> + *
>>> + * 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 MDF_STM32_DFSDM__H
>>> +#define MDF_STM32_DFSDM__H
>>> +
>>> +#include <linux/bitfield.h>
>>> +
>>> +#include <linux/iio/iio.h>
>>> +/*
>>> + * STM32 DFSDM - global register map
>>> + * ________________________________________________________
>>> + * | Offset | Registers block |
>>> + * --------------------------------------------------------
>>> + * | 0x000 | CHANNEL 0 + COMMON CHANNEL FIELDS |
>>> + * --------------------------------------------------------
>>> + * | 0x020 | CHANNEL 1 |
>>> + * --------------------------------------------------------
>>> + * | ... | ..... |
>>> + * --------------------------------------------------------
>>> + * | 0x0E0 | CHANNEL 7 |
>>> + * --------------------------------------------------------
>>> + * | 0x100 | FILTER 0 + COMMON FILTER FIELDs |
>>> + * --------------------------------------------------------
>>> + * | 0x200 | FILTER 1 |
>>> + * --------------------------------------------------------
>>> + * | 0x300 | FILTER 2 |
>>> + * --------------------------------------------------------
>>> + * | 0x400 | FILTER 3 |
>>> + * --------------------------------------------------------
>>> + */
>>> +
>>> +/*
>>> + * Channels register definitions
>>> + */
>>> +#define DFSDM_CHCFGR1(y) ((y) * 0x20 + 0x00)
>>> +#define DFSDM_CHCFGR2(y) ((y) * 0x20 + 0x04)
>>> +#define DFSDM_AWSCDR(y) ((y) * 0x20 + 0x08)
>>> +#define DFSDM_CHWDATR(y) ((y) * 0x20 + 0x0C)
>>> +#define DFSDM_CHDATINR(y) ((y) * 0x20 + 0x10)
>>> +
>>> +/* CHCFGR1: Channel configuration register 1 */
>>> +#define DFSDM_CHCFGR1_SITP_MASK GENMASK(1, 0)
>>> +#define DFSDM_CHCFGR1_SITP(v) FIELD_PREP(DFSDM_CHCFGR1_SITP_MASK, v)
>>> +#define DFSDM_CHCFGR1_SPICKSEL_MASK GENMASK(3, 2)
>>> +#define DFSDM_CHCFGR1_SPICKSEL(v) FIELD_PREP(DFSDM_CHCFGR1_SPICKSEL_MASK, v)
>>> +#define DFSDM_CHCFGR1_SCDEN_MASK BIT(5)
>>> +#define DFSDM_CHCFGR1_SCDEN(v) FIELD_PREP(DFSDM_CHCFGR1_SCDEN_MASK, v)
>>> +#define DFSDM_CHCFGR1_CKABEN_MASK BIT(6)
>>> +#define DFSDM_CHCFGR1_CKABEN(v) FIELD_PREP(DFSDM_CHCFGR1_CKABEN_MASK, v)
>>> +#define DFSDM_CHCFGR1_CHEN_MASK BIT(7)
>>> +#define DFSDM_CHCFGR1_CHEN(v) FIELD_PREP(DFSDM_CHCFGR1_CHEN_MASK, v)
>>> +#define DFSDM_CHCFGR1_CHINSEL_MASK BIT(8)
>>> +#define DFSDM_CHCFGR1_CHINSEL(v) FIELD_PREP(DFSDM_CHCFGR1_CHINSEL_MASK, v)
>>> +#define DFSDM_CHCFGR1_DATMPX_MASK GENMASK(13, 12)
>>> +#define DFSDM_CHCFGR1_DATMPX(v) FIELD_PREP(DFSDM_CHCFGR1_DATMPX_MASK, v)
>>> +#define DFSDM_CHCFGR1_DATPACK_MASK GENMASK(15, 14)
>>> +#define DFSDM_CHCFGR1_DATPACK(v) FIELD_PREP(DFSDM_CHCFGR1_DATPACK_MASK, v)
>>> +#define DFSDM_CHCFGR1_CKOUTDIV_MASK GENMASK(23, 16)
>>> +#define DFSDM_CHCFGR1_CKOUTDIV(v) FIELD_PREP(DFSDM_CHCFGR1_CKOUTDIV_MASK, v)
>>> +#define DFSDM_CHCFGR1_CKOUTSRC_MASK BIT(30)
>>> +#define DFSDM_CHCFGR1_CKOUTSRC(v) FIELD_PREP(DFSDM_CHCFGR1_CKOUTSRC_MASK, v)
>>> +#define DFSDM_CHCFGR1_DFSDMEN_MASK BIT(31)
>>> +#define DFSDM_CHCFGR1_DFSDMEN(v) FIELD_PREP(DFSDM_CHCFGR1_DFSDMEN_MASK, v)
>>> +
>>> +/* CHCFGR2: Channel configuration register 2 */
>>> +#define DFSDM_CHCFGR2_DTRBS_MASK GENMASK(7, 3)
>>> +#define DFSDM_CHCFGR2_DTRBS(v) FIELD_PREP(DFSDM_CHCFGR2_DTRBS_MASK, v)
>>> +#define DFSDM_CHCFGR2_OFFSET_MASK GENMASK(31, 8)
>>> +#define DFSDM_CHCFGR2_OFFSET(v) FIELD_PREP(DFSDM_CHCFGR2_OFFSET_MASK, v)
>>> +
>>> +/* AWSCDR: Channel analog watchdog and short circuit detector */
>>> +#define DFSDM_AWSCDR_SCDT_MASK GENMASK(7, 0)
>>> +#define DFSDM_AWSCDR_SCDT(v) FIELD_PREP(DFSDM_AWSCDR_SCDT_MASK, v)
>>> +#define DFSDM_AWSCDR_BKSCD_MASK GENMASK(15, 12)
>>> +#define DFSDM_AWSCDR_BKSCD(v) FIELD_PREP(DFSDM_AWSCDR_BKSCD_MASK, v)
>>> +#define DFSDM_AWSCDR_AWFOSR_MASK GENMASK(20, 16)
>>> +#define DFSDM_AWSCDR_AWFOSR(v) FIELD_PREP(DFSDM_AWSCDR_AWFOSR_MASK, v)
>>> +#define DFSDM_AWSCDR_AWFORD_MASK GENMASK(23, 22)
>>> +#define DFSDM_AWSCDR_AWFORD(v) FIELD_PREP(DFSDM_AWSCDR_AWFORD_MASK, v)
>>> +
>>> +/*
>>> + * Filters register definitions
>>> + */
>>> +#define DFSDM_FILTER_BASE_ADR 0x100
>>> +#define DFSDM_FILTER_REG_MASK 0x7F
>>> +#define DFSDM_FILTER_X_BASE_ADR(x) ((x) * 0x80 + DFSDM_FILTER_BASE_ADR)
>>> +
>>> +#define DFSDM_CR1(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x00)
>>> +#define DFSDM_CR2(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x04)
>>> +#define DFSDM_ISR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x08)
>>> +#define DFSDM_ICR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x0C)
>>> +#define DFSDM_JCHGR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x10)
>>> +#define DFSDM_FCR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x14)
>>> +#define DFSDM_JDATAR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x18)
>>> +#define DFSDM_RDATAR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x1C)
>>> +#define DFSDM_AWHTR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x20)
>>> +#define DFSDM_AWLTR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x24)
>>> +#define DFSDM_AWSR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x28)
>>> +#define DFSDM_AWCFR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x2C)
>>> +#define DFSDM_EXMAX(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x30)
>>> +#define DFSDM_EXMIN(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x34)
>>> +#define DFSDM_CNVTIMR(x) (DFSDM_FILTER_X_BASE_ADR(x) + 0x38)
>>> +
>>> +/* CR1 Control register 1 */
>>> +#define DFSDM_CR1_DFEN_MASK BIT(0)
>>> +#define DFSDM_CR1_DFEN(v) FIELD_PREP(DFSDM_CR1_DFEN_MASK, v)
>>> +#define DFSDM_CR1_JSWSTART_MASK BIT(1)
>>> +#define DFSDM_CR1_JSWSTART(v) FIELD_PREP(DFSDM_CR1_JSWSTART_MASK, v)
>>> +#define DFSDM_CR1_JSYNC_MASK BIT(3)
>>> +#define DFSDM_CR1_JSYNC(v) FIELD_PREP(DFSDM_CR1_JSYNC_MASK, v)
>>> +#define DFSDM_CR1_JSCAN_MASK BIT(4)
>>> +#define DFSDM_CR1_JSCAN(v) FIELD_PREP(DFSDM_CR1_JSCAN_MASK, v)
>>> +#define DFSDM_CR1_JDMAEN_MASK BIT(5)
>>> +#define DFSDM_CR1_JDMAEN(v) FIELD_PREP(DFSDM_CR1_JDMAEN_MASK, v)
>>> +#define DFSDM_CR1_JEXTSEL_MASK GENMASK(12, 8)
>>> +#define DFSDM_CR1_JEXTSEL(v) FIELD_PREP(DFSDM_CR1_JEXTSEL_MASK, v)
>>> +#define DFSDM_CR1_JEXTEN_MASK GENMASK(14, 13)
>>> +#define DFSDM_CR1_JEXTEN(v) FIELD_PREP(DFSDM_CR1_JEXTEN_MASK, v)
>>> +#define DFSDM_CR1_RSWSTART_MASK BIT(17)
>>> +#define DFSDM_CR1_RSWSTART(v) FIELD_PREP(DFSDM_CR1_RSWSTART_MASK, v)
>>> +#define DFSDM_CR1_RCONT_MASK BIT(18)
>>> +#define DFSDM_CR1_RCONT(v) FIELD_PREP(DFSDM_CR1_RCONT_MASK, v)
>>> +#define DFSDM_CR1_RSYNC_MASK BIT(19)
>>> +#define DFSDM_CR1_RSYNC(v) FIELD_PREP(DFSDM_CR1_RSYNC_MASK, v)
>>> +#define DFSDM_CR1_RDMAEN_MASK BIT(21)
>>> +#define DFSDM_CR1_RDMAEN(v) FIELD_PREP(DFSDM_CR1_RDMAEN_MASK, v)
>>> +#define DFSDM_CR1_RCH_MASK GENMASK(26, 24)
>>> +#define DFSDM_CR1_RCH(v) FIELD_PREP(DFSDM_CR1_RCH_MASK, v)
>>> +#define DFSDM_CR1_FAST_MASK BIT(29)
>>> +#define DFSDM_CR1_FAST(v) FIELD_PREP(DFSDM_CR1_FAST_MASK, v)
>>> +#define DFSDM_CR1_AWFSEL_MASK BIT(30)
>>> +#define DFSDM_CR1_AWFSEL(v) FIELD_PREP(DFSDM_CR1_AWFSEL_MASK, v)
>>> +
>>> +/* CR2: Control register 2 */
>>> +#define DFSDM_CR2_IE_MASK GENMASK(6, 0)
>>> +#define DFSDM_CR2_IE(v) FIELD_PREP(DFSDM_CR2_IE_MASK, v)
>>> +#define DFSDM_CR2_JEOCIE_MASK BIT(0)
>>> +#define DFSDM_CR2_JEOCIE(v) FIELD_PREP(DFSDM_CR2_JEOCIE_MASK, v)
>>> +#define DFSDM_CR2_REOCIE_MASK BIT(1)
>>> +#define DFSDM_CR2_REOCIE(v) FIELD_PREP(DFSDM_CR2_REOCIE_MASK, v)
>>> +#define DFSDM_CR2_JOVRIE_MASK BIT(2)
>>> +#define DFSDM_CR2_JOVRIE(v) FIELD_PREP(DFSDM_CR2_JOVRIE_MASK, v)
>>> +#define DFSDM_CR2_ROVRIE_MASK BIT(3)
>>> +#define DFSDM_CR2_ROVRIE(v) FIELD_PREP(DFSDM_CR2_ROVRIE_MASK, v)
>>> +#define DFSDM_CR2_AWDIE_MASK BIT(4)
>>> +#define DFSDM_CR2_AWDIE(v) FIELD_PREP(DFSDM_CR2_AWDIE_MASK, v)
>>> +#define DFSDM_CR2_SCDIE_MASK BIT(5)
>>> +#define DFSDM_CR2_SCDIE(v) FIELD_PREP(DFSDM_CR2_SCDIE_MASK, v)
>>> +#define DFSDM_CR2_CKABIE_MASK BIT(6)
>>> +#define DFSDM_CR2_CKABIE(v) FIELD_PREP(DFSDM_CR2_CKABIE_MASK, v)
>>> +#define DFSDM_CR2_EXCH_MASK GENMASK(15, 8)
>>> +#define DFSDM_CR2_EXCH(v) FIELD_PREP(DFSDM_CR2_EXCH_MASK, v)
>>> +#define DFSDM_CR2_AWDCH_MASK GENMASK(23, 16)
>>> +#define DFSDM_CR2_AWDCH(v) FIELD_PREP(DFSDM_CR2_AWDCH_MASK, v)
>>> +
>>> +/* ISR: Interrupt status register */
>>> +#define DFSDM_ISR_JEOCF_MASK BIT(0)
>>> +#define DFSDM_ISR_JEOCF(v) FIELD_PREP(DFSDM_ISR_JEOCF_MASK, v)
>>> +#define DFSDM_ISR_REOCF_MASK BIT(1)
>>> +#define DFSDM_ISR_REOCF(v) FIELD_PREP(DFSDM_ISR_REOCF_MASK, v)
>>> +#define DFSDM_ISR_JOVRF_MASK BIT(2)
>>> +#define DFSDM_ISR_JOVRF(v) FIELD_PREP(DFSDM_ISR_JOVRF_MASK, v)
>>> +#define DFSDM_ISR_ROVRF_MASK BIT(3)
>>> +#define DFSDM_ISR_ROVRF(v) FIELD_PREP(DFSDM_ISR_ROVRF_MASK, v)
>>> +#define DFSDM_ISR_AWDF_MASK BIT(4)
>>> +#define DFSDM_ISR_AWDF(v) FIELD_PREP(DFSDM_ISR_AWDF_MASK, v)
>>> +#define DFSDM_ISR_JCIP_MASK BIT(13)
>>> +#define DFSDM_ISR_JCIP(v) FIELD_PREP(DFSDM_ISR_JCIP_MASK, v)
>>> +#define DFSDM_ISR_RCIP_MASK BIT(14)
>>> +#define DFSDM_ISR_RCIP(v) FIELD_PREP(DFSDM_ISR_RCIP, v)
>>> +#define DFSDM_ISR_CKABF_MASK GENMASK(23, 16)
>>> +#define DFSDM_ISR_CKABF(v) FIELD_PREP(DFSDM_ISR_CKABF_MASK, v)
>>> +#define DFSDM_ISR_SCDF_MASK GENMASK(31, 24)
>>> +#define DFSDM_ISR_SCDF(v) FIELD_PREP(DFSDM_ISR_SCDF_MASK, v)
>>> +
>>> +/* ICR: Interrupt flag clear register */
>>> +#define DFSDM_ICR_CLRJOVRF_MASK BIT(2)
>>> +#define DFSDM_ICR_CLRJOVRF(v) FIELD_PREP(DFSDM_ICR_CLRJOVRF_MASK, v)
>>> +#define DFSDM_ICR_CLRROVRF_MASK BIT(3)
>>> +#define DFSDM_ICR_CLRROVRF(v) FIELD_PREP(DFSDM_ICR_CLRROVRF_MASK, v)
>>> +#define DFSDM_ICR_CLRCKABF_MASK GENMASK(23, 16)
>>> +#define DFSDM_ICR_CLRCKABF(v) FIELD_PREP(DFSDM_ICR_CLRCKABF_MASK, v)
>>> +#define DFSDM_ICR_CLRCKABF_CH_MASK(y) BIT(16 + (y))
>>> +#define DFSDM_ICR_CLRCKABF_CH(v, y) \
>>> + (((v) << (16 + (y))) & DFSDM_ICR_CLRCKABF_CH_MASK(y))
>>> +#define DFSDM_ICR_CLRSCDF_MASK GENMASK(31, 24)
>>> +#define DFSDM_ICR_CLRSCDF(v) FIELD_PREP(DFSDM_ICR_CLRSCDF_MASK, v)
>>> +#define DFSDM_ICR_CLRSCDF_CH_MASK(y) BIT(24 + (y))
>>> +#define DFSDM_ICR_CLRSCDF_CH(v, y) \
>>> + (((v) << (24 + (y))) & DFSDM_ICR_CLRSCDF_MASK(y))
>>> +
>>> +/* FCR: Filter control register */
>>> +#define DFSDM_FCR_IOSR_MASK GENMASK(7, 0)
>>> +#define DFSDM_FCR_IOSR(v) FIELD_PREP(DFSDM_FCR_IOSR_MASK, v)
>>> +#define DFSDM_FCR_FOSR_MASK GENMASK(25, 16)
>>> +#define DFSDM_FCR_FOSR(v) FIELD_PREP(DFSDM_FCR_FOSR_MASK, v)
>>> +#define DFSDM_FCR_FORD_MASK GENMASK(31, 29)
>>> +#define DFSDM_FCR_FORD(v) FIELD_PREP(DFSDM_FCR_FORD_MASK, v)
>>> +
>>> +/* RDATAR: Filter data register for regular channel */
>>> +#define DFSDM_DATAR_CH_MASK GENMASK(2, 0)
>>> +#define DFSDM_DATAR_DATA_OFFSET 8
>>> +#define DFSDM_DATAR_DATA_MASK GENMASK(31, DFSDM_DATAR_DATA_OFFSET)
>>> +
>>> +/* AWLTR: Filter analog watchdog low threshold register */
>>> +#define DFSDM_AWLTR_BKAWL_MASK GENMASK(3, 0)
>>> +#define DFSDM_AWLTR_BKAWL(v) FIELD_PREP(DFSDM_AWLTR_BKAWL_MASK, v)
>>> +#define DFSDM_AWLTR_AWLT_MASK GENMASK(31, 8)
>>> +#define DFSDM_AWLTR_AWLT(v) FIELD_PREP(DFSDM_AWLTR_AWLT_MASK, v)
>>> +
>>> +/* AWHTR: Filter analog watchdog low threshold register */
>>> +#define DFSDM_AWHTR_BKAWH_MASK GENMASK(3, 0)
>>> +#define DFSDM_AWHTR_BKAWH(v) FIELD_PREP(DFSDM_AWHTR_BKAWH_MASK, v)
>>> +#define DFSDM_AWHTR_AWHT_MASK GENMASK(31, 8)
>>> +#define DFSDM_AWHTR_AWHT(v) FIELD_PREP(DFSDM_AWHTR_AWHT_MASK, v)
>>> +
>>> +/* AWSR: Filter watchdog status register */
>>> +#define DFSDM_AWSR_AWLTF_MASK GENMASK(7, 0)
>>> +#define DFSDM_AWSR_AWLTF(v) FIELD_PREP(DFSDM_AWSR_AWLTF_MASK, v)
>>> +#define DFSDM_AWSR_AWHTF_MASK GENMASK(15, 8)
>>> +#define DFSDM_AWSR_AWHTF(v) FIELD_PREP(DFSDM_AWSR_AWHTF_MASK, v)
>>> +
>>> +/* AWCFR: Filter watchdog status register */
>>> +#define DFSDM_AWCFR_AWLTF_MASK GENMASK(7, 0)
>>> +#define DFSDM_AWCFR_AWLTF(v) FIELD_PREP(DFSDM_AWCFR_AWLTF_MASK, v)
>>> +#define DFSDM_AWCFR_AWHTF_MASK GENMASK(15, 8)
>>> +#define DFSDM_AWCFR_AWHTF(v) FIELD_PREP(DFSDM_AWCFR_AWHTF_MASK, v)
>>> +
>>> +/* DFSDM filter order */
>>> +enum stm32_dfsdm_sinc_order {
>>> + DFSDM_FASTSINC_ORDER, /* FastSinc filter type */
>>> + DFSDM_SINC1_ORDER, /* Sinc 1 filter type */
>>> + DFSDM_SINC2_ORDER, /* Sinc 2 filter type */
>>> + DFSDM_SINC3_ORDER, /* Sinc 3 filter type */
>>> + DFSDM_SINC4_ORDER, /* Sinc 4 filter type (N.A. for watchdog) */
>>> + DFSDM_SINC5_ORDER, /* Sinc 5 filter type (N.A. for watchdog) */
>>> + DFSDM_NB_SINC_ORDER,
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_dfsdm_filter - structure relative to stm32 FDSDM filter
>>> + * TODO: complete structure.
>> nice :) RFC I guess :)
>>> + * @id: filetr ID,
>>> + */
>>> +struct stm32_dfsdm_filter {
>>> + unsigned int id;
>>> + unsigned int iosr; /* integrator oversampling */
>>> + unsigned int fosr; /* filter oversampling */
>>> + enum stm32_dfsdm_sinc_order ford;
>>> + u64 res; /* output sample resolution */
>>> + unsigned int sync_mode; /* filter suynchronized with filter0 */
>>> + unsigned int fast; /* filter fast mode */
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_dfsdm_channel - structure relative to stm32 FDSDM channel
>>> + * TODO: complete structure.
>>> + * @id: filetr ID,
>> filter
>>> + */
>>> +struct stm32_dfsdm_channel {
>>> + unsigned int id; /* id of the channel */
>>> + unsigned int type; /* interface type linked to stm32_dfsdm_chan_type */
>>> + unsigned int src; /* interface type linked to stm32_dfsdm_chan_src */
>>> + unsigned int alt_si; /* use alternative serial input interface */
>>> +};
>>> +
>>> +/**
>>> + * struct stm32_dfsdm - stm32 FDSDM driver common data (for all instances)
>>> + * @base: control registers base cpu addr
>>> + * @phys_base: DFSDM IP register physical address.
>>> + * @fl_list: filter resources list
>>> + * @num_fl: number of filter resources available
>>> + * @ch_list: channel resources list
>>> + * @num_chs: number of channel resources available
>>> + */
>>> +struct stm32_dfsdm {
>>> + void __iomem *base;
>>> + phys_addr_t phys_base;
>>> + struct regmap *regmap;
>>> + struct stm32_dfsdm_filter *fl_list;
>>> + unsigned int num_fls;
>>> + struct stm32_dfsdm_channel *ch_list;
>>> + unsigned int num_chs;
>>> + unsigned int spi_master_freq;
>>> +};
>>> +
>>> +struct stm32_dfsdm_str2field {
>>> + const char *name;
>>> + unsigned int val;
>>> +};
>>> +
>>> +/* DFSDM channel serial interface type */
>>> +static const struct stm32_dfsdm_str2field stm32_dfsdm_chan_type[] = {
>>> + { "SPI_R", 0 }, /* SPI with data on rising edge */
>>> + { "SPI_F", 1 }, /* SPI with data on falling edge */
>>> + { "MANCH_R", 2 }, /* Manchester codec, rising edge = logic 0 */
>>> + { "MANCH_F", 3 }, /* Manchester codec, falling edge = logic 1 */
>>> + { 0, 0},
>>> +};
>>> +
>>> +/* DFSDM channel serial spi clock source */
>>> +enum stm32_dfsdm_spi_clk_src {
>>> + DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL,
>>> + DFSDM_CHANNEL_SPI_CLOCK_INTERNAL,
>>> + DFSDM_CHANNEL_SPI_CLOCK_INTERNAL_DIV2_FALLING,
>>> + DFSDM_CHANNEL_SPI_CLOCK_INTERNAL_DIV2_RISING
>>> +};
>>> +
>>> +/* DFSDM channel clock source */
>>> +static const struct stm32_dfsdm_str2field stm32_dfsdm_chan_src[] = {
>>> + /* External SPI clock (CLKIN x) */
>>> + { "CLKIN", DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL },
>>> + /* Internal SPI clock (CLKOUT) */
>>> + { "CLKOUT", DFSDM_CHANNEL_SPI_CLOCK_INTERNAL },
>>> + /* Internal SPI clock divided by 2 (falling edge) */
>>> + { "CLKOUT_F", DFSDM_CHANNEL_SPI_CLOCK_INTERNAL_DIV2_FALLING },
>>> + /* Internal SPI clock divided by 2 (falling edge) */
>>> + { "CLKOUT_R", DFSDM_CHANNEL_SPI_CLOCK_INTERNAL_DIV2_RISING },
>>> + { 0, 0 },
>>> +};
>>> +
>>> +/* DFSDM Serial interface trigger name */
>>> +#define DFSDM_SPI_TRIGGER_NAME "DFSDM_SERIAL_IN"
>>> +
>>> +static inline int stm32_dfsdm_str2val(const char *str,
>>> + const struct stm32_dfsdm_str2field *list)
>>> +{
>>> + const struct stm32_dfsdm_str2field *p = list;
>>> +
>>> + for (p = list; p && p->name; p++) {
>>> + if (!strcmp(p->name, str))
>>> + return p->val;
>>> + }
>>> + return -EINVAL;
>>> +}
>>> +
>>> +int stm32_dfsdm_set_osrs(struct stm32_dfsdm_filter *fl, unsigned int fast,
>>> + unsigned int oversamp);
>>> +int stm32_dfsdm_start_dfsdm(struct stm32_dfsdm *dfsdm);
>>> +int stm32_dfsdm_stop_dfsdm(struct stm32_dfsdm *dfsdm);
>>> +
>>> +int stm32_dfsdm_filter_configure(struct stm32_dfsdm *dfsdm, unsigned int fl_id,
>>> + unsigned int ch_id);
>>> +int stm32_dfsdm_start_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id);
>>> +void stm32_dfsdm_stop_filter(struct stm32_dfsdm *dfsdm, unsigned int fl_id);
>>> +
>>> +int stm32_dfsdm_chan_configure(struct stm32_dfsdm *dfsdm,
>>> + struct stm32_dfsdm_channel *ch);
>>> +int stm32_dfsdm_start_channel(struct stm32_dfsdm *dfsdm, unsigned int ch_id);
>>> +void stm32_dfsdm_stop_channel(struct stm32_dfsdm *dfsdm, unsigned int ch_id);
>>> +
>>> +int stm32_dfsdm_channel_parse_of(struct stm32_dfsdm *dfsdm,
>>> + struct iio_dev *indio_dev,
>>> + struct iio_chan_spec *chan, int chan_idx);
>>> +
>>> +#endif
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v3 06/11] IIO: ADC: add stm32 DFSDM support for PDM microphone
From: Jonathan Cameron @ 2017-03-25 15:59 UTC (permalink / raw)
To: Arnaud Pouliquen, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Olivier MOYSAN, kernel@stlinux.com, linux-iio@vger.kernel.org,
Maxime Coquelin, linux-arm-kernel@lists.infradead.org,
Alexandre TORGUE
In-Reply-To: <33699891-e102-1967-f47b-46a22e79a251@st.com>
On 20/03/17 11:29, Arnaud Pouliquen wrote:
> Please find my comments in-line
>
> Thanks and Regards,
> Arnaud
>
> On 03/19/2017 11:38 PM, Jonathan Cameron wrote:
>> On 17/03/17 14:08, Arnaud Pouliquen wrote:
>>> Add DFSDM driver to handle PDM audio microphones.
>>>
>>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
>> So key element here is that we really need to have a proper way of
>> doing DMA buffer consumers from IIO (as you said in your cover letter).
>>
>> Not entirely obvious how to do this. Would like some input from Lars
>> if possible. I'm afraid I don't have an suitable hardware to try anything
>> out on (or the time really!). This will obviously be quite different from
>> the single scan stuff you have seen is there at the moment.
> I saw 2 other drivers ti_am335x_adc.c and stm32_adc.c) that use cyclic
> DMA. I suppose that problematics are similar. Perhaps the extra
> constrains in DFSDM is the in-kernel API used to get the data.
Absolutely. Cyclic dma is becoming more and more common on SoC ADCs.
There are a few more fpga based ones in Analog devices tree as well -
most of those actually do the dma buffer route rather than pushing through
the normal buffer interface (kfifo effectively). Right now we just
don't have any means of accessing these dma buffers in kernel.
Unlike the normal consumer interface, I doubt we'll want to do demuxing
of the stream for this sort of usecase - feels like exclusive use is more
likely.
>
>>
>> Whether this whole approach makes sense vs doing the dma in the alsa driver
>> isn't clear to me.
>>
>> On the plus side, presumably we'll get love dma ADC support for free
>> as part of doing it this way ;)
>>
>> It's been a while since I looked at the IIO dma buffer stuff at all. Will
>> try and refresh my memory sometime this week.
>
> Ok, i will wait your feedback (and Lars's one) before updating my code
> for next version.
It may well be the case that it will take us sometime (when I say us, I mean
you and Lars ;) to pin down how to do dma buffer consumers, so it may be
a case of taking the driver in a state close to the current one with the
intent to switch to a generic way of handling it later.
As long as bindings and userspace don't change we can of course mess with
anything we like.
Jonathan
>>
>> Jonathan
>>> ---
>>> drivers/iio/adc/Kconfig | 13 +
>>> drivers/iio/adc/Makefile | 1 +
>>> drivers/iio/adc/stm32-dfsdm-audio.c | 720 ++++++++++++++++++++++++++++++
>>> include/linux/iio/adc/stm32-dfsdm-audio.h | 41 ++
>>> 4 files changed, 775 insertions(+)
>>> create mode 100644 drivers/iio/adc/stm32-dfsdm-audio.c
>>> create mode 100644 include/linux/iio/adc/stm32-dfsdm-audio.h
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 3e0eb11..c108933 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -478,6 +478,19 @@ config STM32_DFSDM_ADC
>>> This driver can also be built as a module. If so, the module
>>> will be called stm32-dfsdm-adc.
>>>
>>> +config STM32_DFSDM_AUDIO
>>> + tristate "STMicroelectronics STM32 dfsdm audio
>>> + depends on (ARCH_STM32 && OF) || COMPILE_TEST
>>> + select STM32_DFSDM_CORE
>>> + select REGMAP_MMIO
>>> + select IIO_BUFFER_DMAENGINE
>>> + help
>>> + Select this option to support Audio PDM micophone for
>>> + STMicroelectronics STM32 digital filter for sigma delta converter.
>>> +
>>> + This driver can also be built as a module. If so, the module
>>> + will be called stm32-dfsdm-audio.
>>> +
>>> config STX104
>>> tristate "Apex Embedded Systems STX104 driver"
>>> depends on X86 && ISA_BUS_API
>>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>>> index 161f271..79f975d 100644
>>> --- a/drivers/iio/adc/Makefile
>>> +++ b/drivers/iio/adc/Makefile
>>> @@ -44,6 +44,7 @@ obj-$(CONFIG_STX104) += stx104.o
>>> obj-$(CONFIG_STM32_ADC_CORE) += stm32-adc-core.o
>>> obj-$(CONFIG_STM32_ADC) += stm32-adc.o
>>> obj-$(CONFIG_STM32_DFSDM_ADC) += stm32-dfsdm-adc.o
>>> +obj-$(CONFIG_STM32_DFSDM_AUDIO) += stm32-dfsdm-audio.o
>>> obj-$(CONFIG_STM32_DFSDM_CORE) += stm32-dfsdm-core.o
>>> obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
>>> obj-$(CONFIG_TI_ADC0832) += ti-adc0832.o
>>> diff --git a/drivers/iio/adc/stm32-dfsdm-audio.c b/drivers/iio/adc/stm32-dfsdm-audio.c
>>> new file mode 100644
>>> index 0000000..115ef8f
>>> --- /dev/null
>>> +++ b/drivers/iio/adc/stm32-dfsdm-audio.c
>>> @@ -0,0 +1,720 @@
>>> +/*
>>> + * This file is the ADC part of of the STM32 DFSDM driver
>>> + *
>>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>>> + * Author: Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>>> + *
>>> + * License type: GPLv2
>>> + *
>>> + * 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.
>>> + *
>>> + * You should have received a copy of the GNU General Public License along with
>>> + * this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/dmaengine.h>
>>> +#include <linux/dma-mapping.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regmap.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/hw_consumer.h>
>>> +#include <linux/iio/sysfs.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>> +
>>> +#include "stm32-dfsdm.h"
>>> +
>>> +#define DFSDM_DMA_BUFFER_SIZE (4 * PAGE_SIZE)
>>> +
>>> +struct stm32_dfsdm_audio {
>>> + struct stm32_dfsdm *dfsdm;
>>> + unsigned int fl_id;
>>> + unsigned int ch_id;
>>> + unsigned int spi_freq; /* SPI bus clock frequency */
>>> + unsigned int sample_freq; /* Sample frequency after filter decimation */
>>> +
>>> + u8 *rx_buf;
>>> + unsigned int bufi; /* Buffer current position */
>>> + unsigned int buf_sz; /* Buffer size */
>>> +
>>> + struct dma_chan *dma_chan;
>>> + dma_addr_t dma_buf;
>>> +
>>> + int (*cb)(const void *data, size_t size, void *cb_priv);
>>> + void *cb_priv;
>>> +};
>>> +
>>> +const char *stm32_dfsdm_spi_trigger = DFSDM_SPI_TRIGGER_NAME;
>>> +
>>> +static ssize_t dfsdm_audio_get_rate(struct iio_dev *indio_dev, uintptr_t priv,
>>> + const struct iio_chan_spec *chan, char *buf)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> +
>>> + return snprintf(buf, PAGE_SIZE, "%d\n", pdmc->sample_freq);
>>> +}
>>> +
>>> +static ssize_t dfsdm_audio_set_rate(struct iio_dev *indio_dev, uintptr_t priv,
>>> + const struct iio_chan_spec *chan,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + struct stm32_dfsdm_filter *fl = &pdmc->dfsdm->fl_list[pdmc->fl_id];
>>> + struct stm32_dfsdm_channel *ch = &pdmc->dfsdm->ch_list[pdmc->ch_id];
>>> + unsigned int spi_freq = pdmc->spi_freq;
>>> + unsigned int sample_freq;
>>> + int ret;
>>> +
>>> + ret = kstrtoint(buf, 0, &sample_freq);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + dev_dbg(&indio_dev->dev, "Requested sample_freq :%d\n", sample_freq);
>>> + if (!sample_freq)
>>> + return -EINVAL;
>>> +
>>> + if (ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
>>> + spi_freq = pdmc->dfsdm->spi_master_freq;
>>> +
>>> + if (spi_freq % sample_freq)
>>> + dev_warn(&indio_dev->dev, "Sampling rate not accurate (%d)\n",
>>> + spi_freq / (spi_freq / sample_freq));
>>> +
>>> + ret = stm32_dfsdm_set_osrs(fl, 0, (spi_freq / sample_freq));
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + "Not able to find filter parameter that match!\n");
>>> + return ret;
>>> + }
>>> + pdmc->sample_freq = sample_freq;
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static ssize_t dfsdm_audio_get_spiclk(struct iio_dev *indio_dev, uintptr_t priv,
>>> + const struct iio_chan_spec *chan,
>>> + char *buf)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> +
>>> + return snprintf(buf, PAGE_SIZE, "%d\n", pdmc->spi_freq);
>>> +}
>>> +
>>> +static ssize_t dfsdm_audio_set_spiclk(struct iio_dev *indio_dev, uintptr_t priv,
>>> + const struct iio_chan_spec *chan,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + struct stm32_dfsdm_filter *fl = &pdmc->dfsdm->fl_list[pdmc->fl_id];
>>> + struct stm32_dfsdm_channel *ch = &pdmc->dfsdm->ch_list[pdmc->ch_id];
>>> + unsigned int sample_freq = pdmc->sample_freq;
>>> + unsigned int spi_freq;
>>> + int ret;
>>> +
>>> + /* If DFSDM is master on SPI, SPI freq can not be updated */
>>> + if (ch->src != DFSDM_CHANNEL_SPI_CLOCK_EXTERNAL)
>>> + return -EPERM;
>>> +
>>> + ret = kstrtoint(buf, 0, &spi_freq);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + dev_dbg(&indio_dev->dev, "Requested frequency :%d\n", spi_freq);
>>> + if (!spi_freq)
>>> + return -EINVAL;
>>> +
>>> + if (sample_freq) {
>>> + if (spi_freq % sample_freq)
>>> + dev_warn(&indio_dev->dev,
>>> + "Sampling rate not accurate (%d)\n",
>>> + spi_freq / (spi_freq / sample_freq));
>>> +
>>> + ret = stm32_dfsdm_set_osrs(fl, 0, (spi_freq / sample_freq));
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev,
>>> + "No filter parameters that match!\n");
>>> + return ret;
>>> + }
>>> + }
>>> + pdmc->spi_freq = spi_freq;
>>> +
>>> + return len;
>>> +}
>>> +
>>> +/*
>>> + * Define external info for SPI Frequency and audio sampling rate that can be
>>> + * configured by ASoC driver through consumer.h API
>>> + */
>>> +static const struct iio_chan_spec_ext_info dfsdm_adc_ext_info[] = {
>>> + /* filter oversampling: Post filter oversampling ratio */
>>> + {
>>> + .name = "audio_sampling_rate",
>>> + .shared = IIO_SHARED_BY_TYPE,
>>> + .read = dfsdm_audio_get_rate,
>>> + .write = dfsdm_audio_set_rate,
>>> + },
>>> + /* data_right_bit_shift : Filter output data shifting */
>>> + {
>>> + .name = "spi_clk_freq",
>>> + .shared = IIO_SHARED_BY_TYPE,
>>> + .read = dfsdm_audio_get_spiclk,
>>> + .write = dfsdm_audio_set_spiclk,
>>> + },
>>> + {},
>>> +};
>>> +
>>> +static int stm32_dfsdm_start_conv(struct stm32_dfsdm_audio *pdmc, bool single)
>>> +{
>>> + struct regmap *regmap = pdmc->dfsdm->regmap;
>>> + int ret;
>>> +
>>> + ret = stm32_dfsdm_start_dfsdm(pdmc->dfsdm);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = stm32_dfsdm_start_channel(pdmc->dfsdm, pdmc->ch_id);
>>> + if (ret < 0)
>>> + goto stop_dfsdm;
>>> +
>>> + ret = stm32_dfsdm_filter_configure(pdmc->dfsdm, pdmc->fl_id,
>>> + pdmc->ch_id);
>>> + if (ret < 0)
>>> + goto stop_channels;
>>> +
>>> + /* Enable DMA transfer*/
>>> + ret = regmap_update_bits(regmap, DFSDM_CR1(pdmc->fl_id),
>>> + DFSDM_CR1_RDMAEN_MASK, DFSDM_CR1_RDMAEN(1));
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + /* Enable conversion triggered by SPI clock*/
>>> + ret = regmap_update_bits(regmap, DFSDM_CR1(pdmc->fl_id),
>>> + DFSDM_CR1_RCONT_MASK, DFSDM_CR1_RCONT(1));
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = stm32_dfsdm_start_filter(pdmc->dfsdm, pdmc->fl_id);
>>> + if (ret < 0)
>>> + goto stop_channels;
>>> +
>>> + return 0;
>>> +
>>> +stop_channels:
>>> + stm32_dfsdm_stop_channel(pdmc->dfsdm, pdmc->fl_id);
>>> +stop_dfsdm:
>>> + stm32_dfsdm_stop_dfsdm(pdmc->dfsdm);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void stm32_dfsdm_stop_conv(struct stm32_dfsdm_audio *pdmc)
>>> +{
>>> + stm32_dfsdm_stop_filter(pdmc->dfsdm, pdmc->fl_id);
>>> +
>>> + stm32_dfsdm_stop_channel(pdmc->dfsdm, pdmc->ch_id);
>>> +
>>> + stm32_dfsdm_stop_dfsdm(pdmc->dfsdm);
>>> +}
>>> +
>>> +static int stm32_dfsdm_set_watermark(struct iio_dev *indio_dev,
>>> + unsigned int val)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + unsigned int watermark = DFSDM_DMA_BUFFER_SIZE / 2;
>>> +
>>> + /*
>>> + * DMA cyclic transfers are used, buffer is split into two periods.
>>> + * There should be :
>>> + * - always one buffer (period) DMA is working on
>>> + * - one buffer (period) driver pushed to ASoC side ().
>>> + */
>>> + watermark = min(watermark, val * (unsigned int)(sizeof(u32)));
>>> + pdmc->buf_sz = watermark * 2;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int stm32_dfsdm_validate_trigger(struct iio_dev *indio_dev,
>>> + struct iio_trigger *trig)
>>> +{
>>> + if (!strcmp(stm32_dfsdm_spi_trigger, trig->name))
>>> + return 0;
>>> +
>>> + return -EINVAL;
>>> +}
>>> +
>>> +static const struct iio_info stm32_dfsdm_info_pdmc = {
>>> + .hwfifo_set_watermark = stm32_dfsdm_set_watermark,
>>> + .driver_module = THIS_MODULE,
>>> + .validate_trigger = stm32_dfsdm_validate_trigger,
>>> +};
>>> +
>>> +static irqreturn_t stm32_dfsdm_irq(int irq, void *arg)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = arg;
>>> + struct iio_dev *indio_dev = iio_priv_to_dev(pdmc);
>>> + struct regmap *regmap = pdmc->dfsdm->regmap;
>>> + unsigned int status;
>>> +
>>> + regmap_read(regmap, DFSDM_ISR(pdmc->fl_id), &status);
>>> +
>>> + if (status & DFSDM_ISR_ROVRF_MASK) {
>>> + dev_err(&indio_dev->dev, "Unexpected Conversion overflow\n");
>>> + regmap_update_bits(regmap, DFSDM_ICR(pdmc->fl_id),
>>> + DFSDM_ICR_CLRROVRF_MASK,
>>> + DFSDM_ICR_CLRROVRF_MASK);
>>> + }
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static unsigned int stm32_dfsdm_audio_avail_data(struct stm32_dfsdm_audio *pdmc)
>>> +{
>>> + struct dma_tx_state state;
>>> + enum dma_status status;
>>> +
>>> + status = dmaengine_tx_status(pdmc->dma_chan,
>>> + pdmc->dma_chan->cookie,
>>> + &state);
>>> + if (status == DMA_IN_PROGRESS) {
>>> + /* Residue is size in bytes from end of buffer */
>>> + unsigned int i = pdmc->buf_sz - state.residue;
>>> + unsigned int size;
>>> +
>>> + /* Return available bytes */
>>> + if (i >= pdmc->bufi)
>>> + size = i - pdmc->bufi;
>>> + else
>>> + size = pdmc->buf_sz + i - pdmc->bufi;
>>> +
>>> + return size;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void stm32_dfsdm_audio_dma_buffer_done(void *data)
>>> +{
>>> + struct iio_dev *indio_dev = data;
>>> +
>>> + iio_trigger_poll_chained(indio_dev->trig);
>> This shouldn't be going through the trigger infrastructure at all really.
>> I'm not 100% sure what doing so is gaining you...
> Yes i will clean trigger part and call directly the ASoC callback here.
>>> +}
>>> +
>>> +static int stm32_dfsdm_audio_dma_start(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + struct dma_async_tx_descriptor *desc;
>>> + dma_cookie_t cookie;
>>> + int ret;
>>> +
>>> + if (!pdmc->dma_chan)
>>> + return -EINVAL;
>>> +
>>> + dev_dbg(&indio_dev->dev, "%s size=%d watermark=%d\n", __func__,
>>> + pdmc->buf_sz, pdmc->buf_sz / 2);
>>> +
>>> + /* Prepare a DMA cyclic transaction */
>>> + desc = dmaengine_prep_dma_cyclic(pdmc->dma_chan,
>>> + pdmc->dma_buf,
>>> + pdmc->buf_sz, pdmc->buf_sz / 2,
>>> + DMA_DEV_TO_MEM,
>>> + DMA_PREP_INTERRUPT);
>>> + if (!desc)
>>> + return -EBUSY;
>>> +
>>> + desc->callback = stm32_dfsdm_audio_dma_buffer_done;
>>> + desc->callback_param = indio_dev;
>>> +
>>> + cookie = dmaengine_submit(desc);
>>> + ret = dma_submit_error(cookie);
>>> + if (ret) {
>>> + dmaengine_terminate_all(pdmc->dma_chan);
>>> + return ret;
>>> + }
>>> +
>>> + /* Issue pending DMA requests */
>>> + dma_async_issue_pending(pdmc->dma_chan);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + dev_dbg(&indio_dev->dev, "%s\n", __func__);
>>> + /* Reset pdmc buffer index */
>>> + pdmc->bufi = 0;
>>> +
>>> + ret = stm32_dfsdm_start_conv(pdmc, false);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Can't start conversion\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = stm32_dfsdm_audio_dma_start(indio_dev);
>>> + if (ret) {
>>> + dev_err(&indio_dev->dev, "Can't start DMA\n");
>>> + goto err_stop_conv;
>>> + }
>>> +
>>> + ret = iio_triggered_buffer_postenable(indio_dev);
>>> + if (ret < 0) {
>>> + dev_err(&indio_dev->dev, "%s :%d\n", __func__, __LINE__);
>>> + goto err_stop_dma;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +err_stop_dma:
>>> + if (pdmc->dma_chan)
>>> + dmaengine_terminate_all(pdmc->dma_chan);
>>> +err_stop_conv:
>>> + stm32_dfsdm_stop_conv(pdmc);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + dev_dbg(&indio_dev->dev, "%s\n", __func__);
>>> + ret = iio_triggered_buffer_predisable(indio_dev);
>>> + if (ret < 0)
>>> + dev_err(&indio_dev->dev, "Predisable failed\n");
>>> +
>>> + if (pdmc->dma_chan)
>>> + dmaengine_terminate_all(pdmc->dma_chan);
>>> +
>>> + stm32_dfsdm_stop_conv(pdmc);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct iio_buffer_setup_ops stm32_dfsdm_buffer_setup_ops = {
>>> + .postenable = &stm32_dfsdm_postenable,
>>> + .predisable = &stm32_dfsdm_predisable,
>>> +};
>>> +
>>> +static irqreturn_t stm32_dfsdm_audio_trigger_handler(int irq, void *p)
>>> +{
>> OK. So now I kind of understand what the trigger provided in the adc driver
>> was about. hmm. Triggers are supposed to be per sample so this is indeed
>> a hack. We need fullblown DMA buffer consumers to do this right.
> Yes exactly.
>>
>> Probably best person to comment on that is Lars.
>>> + struct iio_poll_func *pf = p;
>>> + struct iio_dev *indio_dev = pf->indio_dev;
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + size_t old_pos;
>>> + int available = stm32_dfsdm_audio_avail_data(pdmc);
>>> +
>>> + /*
>>> + * Buffer interface is not support cyclic DMA buffer,and offer only
>>> + * an interface to push data samples per samples.
>>> + * For this reason iio_push_to_buffers_with_timestamp in not used
>>> + * and interface is hacked using a private callback registered by ASoC.
>>> + * This should be a temporary solution waiting a cyclic DMA engine
>>> + * support in IIO.
>>> + */
>>> +
>>> + dev_dbg(&indio_dev->dev, "%s: pos = %d, available = %d\n", __func__,
>>> + pdmc->bufi, available);
>>> + old_pos = pdmc->bufi;
>>> + while (available >= indio_dev->scan_bytes) {
>>> + u32 *buffer = (u32 *)&pdmc->rx_buf[pdmc->bufi];
>>> +
>>> + /* Mask 8 LSB that contains the channel ID */
>>> + *buffer &= 0xFFFFFF00;
>>> + available -= indio_dev->scan_bytes;
>>> + pdmc->bufi += indio_dev->scan_bytes;
>>> + if (pdmc->bufi >= pdmc->buf_sz) {
>>> + if (pdmc->cb)
>>> + pdmc->cb(&pdmc->rx_buf[old_pos],
>>> + pdmc->buf_sz - old_pos, pdmc->cb_priv);
>>> + pdmc->bufi = 0;
>>> + old_pos = 0;
>>> + }
>>> + }
>>> + if (pdmc->cb)
>>> + pdmc->cb(&pdmc->rx_buf[old_pos], pdmc->bufi - old_pos,
>>> + pdmc->cb_priv);
>>> +
>>> + iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +/**
>>> + * stm32_dfsdm_get_buff_cb - register a callback
>>> + * that will be called when DMA transfer period is achieved.
>>> + *
>>> + * @iio_dev: Handle to IIO device.
>>> + * @cb: pointer to callback function.
>>> + * @data: pointer to data buffer
>>> + * @size: size in byte of the data buffer
>>> + * @private: pointer to consumer private structure
>>> + * @private: pointer to consumer private structure
>>> + */
>>> +int stm32_dfsdm_get_buff_cb(struct iio_dev *iio_dev,
>>> + int (*cb)(const void *data, size_t size,
>>> + void *private),
>>> + void *private)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc;
>>> +
>>> + if (!iio_dev)
>>> + return -EINVAL;
>>> + pdmc = iio_priv(iio_dev);
>>> +
>>> + if (iio_dev != iio_priv_to_dev(pdmc))
>>> + return -EINVAL;
>>> +
>>> + pdmc->cb = cb;
>>> + pdmc->cb_priv = private;
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stm32_dfsdm_get_buff_cb);
>>> +
>>> +/**
>>> + * stm32_dfsdm_release_buff_cb - unregister buffer callback
>>> + *
>>> + * @iio_dev: Handle to IIO device.
>>> + */
>>> +int stm32_dfsdm_release_buff_cb(struct iio_dev *iio_dev)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc;
>>> +
>>> + if (!iio_dev)
>>> + return -EINVAL;
>>> + pdmc = iio_priv(iio_dev);
>>> +
>>> + if (iio_dev != iio_priv_to_dev(pdmc))
>>> + return -EINVAL;
>>> + pdmc->cb = NULL;
>>> + pdmc->cb_priv = NULL;
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stm32_dfsdm_release_buff_cb);
>>> +
>>> +static int stm32_dfsdm_audio_chan_init(struct iio_dev *indio_dev)
>>> +{
>>> + struct iio_chan_spec *ch;
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + ch = devm_kzalloc(&indio_dev->dev, sizeof(*ch), GFP_KERNEL);
>>> + if (!ch)
>>> + return -ENOMEM;
>>> +
>>> + ret = stm32_dfsdm_channel_parse_of(pdmc->dfsdm, indio_dev, ch, 0);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ch->type = IIO_VOLTAGE;
>>> + ch->indexed = 1;
>>> + ch->scan_index = 0;
>>> + ch->ext_info = dfsdm_adc_ext_info;
>>> +
>>> + ch->scan_type.sign = 's';
>>> + ch->scan_type.realbits = 24;
>>> + ch->scan_type.storagebits = 32;
>>> +
>>> + pdmc->ch_id = ch->channel;
>>> + ret = stm32_dfsdm_chan_configure(pdmc->dfsdm,
>>> + &pdmc->dfsdm->ch_list[ch->channel]);
>>> +
>>> + indio_dev->num_channels = 1;
>>> + indio_dev->channels = ch;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static const struct of_device_id stm32_dfsdm_audio_match[] = {
>>> + { .compatible = "st,stm32-dfsdm-audio"},
>>> + {}
>>> +};
>>> +
>>> +static int stm32_dfsdm_audio_dma_request(struct iio_dev *indio_dev)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = iio_priv(indio_dev);
>>> + struct dma_slave_config config;
>>> + int ret;
>>> +
>>> + pdmc->dma_chan = dma_request_slave_channel(&indio_dev->dev, "rx");
>>> + if (!pdmc->dma_chan)
>>> + return -EINVAL;
>>> +
>>> + pdmc->rx_buf = dma_alloc_coherent(pdmc->dma_chan->device->dev,
>>> + DFSDM_DMA_BUFFER_SIZE,
>>> + &pdmc->dma_buf, GFP_KERNEL);
>>> + if (!pdmc->rx_buf) {
>>> + ret = -ENOMEM;
>>> + goto err_release;
>>> + }
>>> +
>>> + /* Configure DMA channel to read data register */
>>> + memset(&config, 0, sizeof(config));
>>> + config.src_addr = (dma_addr_t)pdmc->dfsdm->phys_base;
>>> + config.src_addr += DFSDM_RDATAR(pdmc->fl_id);
>>> + config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>>> +
>>> + ret = dmaengine_slave_config(pdmc->dma_chan, &config);
>>> + if (ret)
>>> + goto err_free;
>>> +
>>> + return 0;
>>> +
>>> +err_free:
>>> + dma_free_coherent(pdmc->dma_chan->device->dev, DFSDM_DMA_BUFFER_SIZE,
>>> + pdmc->rx_buf, pdmc->dma_buf);
>>> +err_release:
>>> + dma_release_channel(pdmc->dma_chan);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int stm32_dfsdm_audio_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct stm32_dfsdm_audio *pdmc;
>>> + struct device_node *np = dev->of_node;
>>> + struct iio_dev *iio;
>>> + char *name;
>>> + int ret, irq, val;
>>> +
>>> + iio = devm_iio_device_alloc(dev, sizeof(*pdmc));
>>> + if (IS_ERR(iio)) {
>>> + dev_err(dev, "%s: Failed to allocate IIO\n", __func__);
>>> + return PTR_ERR(iio);
>>> + }
>>> +
>>> + pdmc = iio_priv(iio);
>>> + if (IS_ERR(pdmc)) {
>>> + dev_err(dev, "%s: Failed to allocate ADC\n", __func__);
>>> + return PTR_ERR(pdmc);
>>> + }
>>> + pdmc->dfsdm = dev_get_drvdata(dev->parent);
>>> +
>>> + iio->name = np->name;
>>> + iio->dev.parent = dev;
>>> + iio->dev.of_node = np;
>>> + iio->info = &stm32_dfsdm_info_pdmc;
>>> + iio->modes = INDIO_DIRECT_MODE;
>>> +
>>> + platform_set_drvdata(pdev, pdmc);
>>> +
>>> + ret = of_property_read_u32(dev->of_node, "reg", &pdmc->fl_id);
>>> + if (ret != 0) {
>>> + dev_err(dev, "Missing reg property\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + name = kzalloc(sizeof("dfsdm-pdm0"), GFP_KERNEL);
>>> + if (!name)
>>> + return -ENOMEM;
>>> + snprintf(name, sizeof("dfsdm-pdm0"), "dfsdm-pdm%d", pdmc->fl_id);
>>> + iio->name = name;
>>> +
>>> + /*
>>> + * In a first step IRQs generated for channels are not treated.
>>> + * So IRQ associated to filter instance 0 is dedicated to the Filter 0.
>>> + */
>>> + irq = platform_get_irq(pdev, 0);
>>> + ret = devm_request_irq(dev, irq, stm32_dfsdm_irq,
>>> + 0, pdev->name, pdmc);
>>> + if (ret < 0) {
>>> + dev_err(dev, "Failed to request IRQ\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = of_property_read_u32(dev->of_node, "st,filter-order", &val);
>>> + if (ret < 0) {
>>> + dev_err(dev, "Failed to set filter order\n");
>>> + return ret;
>>> + }
>>> + pdmc->dfsdm->fl_list[pdmc->fl_id].ford = val;
>>> +
>>> + ret = of_property_read_u32(dev->of_node, "st,filter0-sync", &val);
>>> + if (!ret)
>>> + pdmc->dfsdm->fl_list[pdmc->fl_id].sync_mode = val;
>>> +
>>> + ret = stm32_dfsdm_audio_chan_init(iio);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = stm32_dfsdm_audio_dma_request(iio);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "DMA request failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + iio->modes |= INDIO_BUFFER_SOFTWARE;
>>> +
>>> + ret = iio_triggered_buffer_setup(iio,
>>> + &iio_pollfunc_store_time,
>>> + &stm32_dfsdm_audio_trigger_handler,
>>> + &stm32_dfsdm_buffer_setup_ops);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "Buffer setup failed\n");
>>> + goto err_dma_disable;
>>> + }
>>> +
>>> + ret = iio_device_register(iio);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "IIO dev register failed\n");
>>> + goto err_buffer_cleanup;
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +err_buffer_cleanup:
>>> + iio_triggered_buffer_cleanup(iio);
>>> +
>>> +err_dma_disable:
>>> + if (pdmc->dma_chan) {
>>> + dma_free_coherent(pdmc->dma_chan->device->dev,
>>> + DFSDM_DMA_BUFFER_SIZE,
>>> + pdmc->rx_buf, pdmc->dma_buf);
>>> + dma_release_channel(pdmc->dma_chan);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int stm32_dfsdm_audio_remove(struct platform_device *pdev)
>>> +{
>>> + struct stm32_dfsdm_audio *pdmc = platform_get_drvdata(pdev);
>>> + struct iio_dev *iio = iio_priv_to_dev(pdmc);
>>> +
>>> + iio_device_unregister(iio);
>> Same as in the other driver. Can just use the devm_iio_device_register
>> version and drop remove.
> oops missing the previous comment... sorry.
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct platform_driver stm32_dfsdm_audio_driver = {
>>> + .driver = {
>>> + .name = "stm32-dfsdm-audio",
>>> + .of_match_table = stm32_dfsdm_audio_match,
>>> + },
>>> + .probe = stm32_dfsdm_audio_probe,
>>> + .remove = stm32_dfsdm_audio_remove,
>>> +};
>>> +module_platform_driver(stm32_dfsdm_audio_driver);
>>> +
>>> +MODULE_DESCRIPTION("STM32 sigma delta converter for PDM microphone");
>>> +MODULE_AUTHOR("Arnaud Pouliquen <arnaud.pouliquen@st.com>");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/include/linux/iio/adc/stm32-dfsdm-audio.h b/include/linux/iio/adc/stm32-dfsdm-audio.h
>>> new file mode 100644
>>> index 0000000..9b41b28
>>> --- /dev/null
>>> +++ b/include/linux/iio/adc/stm32-dfsdm-audio.h
>>> @@ -0,0 +1,41 @@
>>> +/*
>>> + * This file discribe the STM32 DFSDM IIO driver API for audio part
>>> + *
>>> + * Copyright (C) 2017, STMicroelectronics - All Rights Reserved
>>> + * Author(s): Arnaud Pouliquen <arnaud.pouliquen@st.com>.
>>> + *
>>> + * License terms: GPL V2.0.
>>> + *
>>> + * 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 STM32_DFSDM_AUDIO_H
>>> +#define STM32_DFSDM_AUDIO_H
>>> +
>>> +/**
>>> + * stm32_dfsdm_get_buff_cb() - register callback for capture buffer period.
>>> + * @dev: Pointer to client device.
>>> + * @indio_dev: Device on which the channel exists.
>>> + * @cb: Callback function.
>>> + * @data: pointer to data buffer
>>> + * @size: size of the data buffer in bytes
>>> + * @private: Private data passed to callback.
>>> + *
>>> + */
>>> +int stm32_dfsdm_get_buff_cb(struct iio_dev *iio_dev,
>>> + int (*cb)(const void *data, size_t size,
>>> + void *private),
>>> + void *private);
>>> +/**
>>> + * stm32_dfsdm_get_buff_cb() - release callback for capture buffer period.
>>> + * @indio_dev: Device on which the channel exists.
>>> + */
>>> +int stm32_dfsdm_release_buff_cb(struct iio_dev *iio_dev);
>>> +
>>> +#endif
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v3 07/11] IIO: consumer: allow to set buffer sizes
From: Jonathan Cameron @ 2017-03-25 16:01 UTC (permalink / raw)
To: Arnaud Pouliquen, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Jaroslav Kysela,
Takashi Iwai, Liam Girdwood, Mark Brown
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
Olivier MOYSAN, kernel@stlinux.com, linux-iio@vger.kernel.org,
Maxime Coquelin, linux-arm-kernel@lists.infradead.org,
Alexandre TORGUE
In-Reply-To: <b2715233-63e7-4c9d-49fc-ad6ca8be0d75@st.com>
On 20/03/17 11:30, Arnaud Pouliquen wrote:
>
>
> On 03/19/2017 11:44 PM, Jonathan Cameron wrote:
>> On 17/03/17 14:08, Arnaud Pouliquen wrote:
>>> Add iio consumer API to set buffer size and watermark according
>>> to sysfs API.
>>>
>>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
>> Hmm. Not keen on the length one. Setting a requested watermark
>> is fair enough. There is no actually buffer in these cases though
>> so setting it's length is downright odd..
> Length and watermark are configurable from user land through sysfs.
> Seems to me logic to also propose it in inkern API...
> But I can clean length , no problem.
The concept of length for the consumer interface doesn't currently
make sense as it refers to an actual buffer. The consumer interface
currently passes one entry at a time...
>
>>
>> Guess this is part of the hacks we need to clean up by doing
>> the dma buffer consumer stuff right...
> Yes all is linked :-).
>
>>
>> Jonathan
>>> ---
>>> drivers/iio/buffer/industrialio-buffer-cb.c | 12 ++++++++++++
>>> include/linux/iio/consumer.h | 13 +++++++++++++
>>> 2 files changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/iio/buffer/industrialio-buffer-cb.c b/drivers/iio/buffer/industrialio-buffer-cb.c
>>> index b8f550e..43c066a 100644
>>> --- a/drivers/iio/buffer/industrialio-buffer-cb.c
>>> +++ b/drivers/iio/buffer/industrialio-buffer-cb.c
>>> @@ -103,6 +103,18 @@ struct iio_cb_buffer *iio_channel_get_all_cb(struct device *dev,
>>> }
>>> EXPORT_SYMBOL_GPL(iio_channel_get_all_cb);
>>>
>>> +int iio_channel_cb_set_buffer_params(struct iio_cb_buffer *cb_buff,
>>> + size_t length, size_t watermark)
>>> +{
>>> + if (!length || length < watermark)
>>> + return -EINVAL;
>>> + cb_buff->buffer.watermark = watermark;
>>> + cb_buff->buffer.length = length;
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(iio_channel_start_all_cb);
>>> +
>>> int iio_channel_start_all_cb(struct iio_cb_buffer *cb_buff)
>>> {
>>> return iio_update_buffers(cb_buff->indio_dev, &cb_buff->buffer,
>>> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
>>> index cb44771..0f6e94d 100644
>>> --- a/include/linux/iio/consumer.h
>>> +++ b/include/linux/iio/consumer.h
>>> @@ -134,6 +134,19 @@ struct iio_cb_buffer *iio_channel_get_all_cb(struct device *dev,
>>> void *private),
>>> void *private);
>>> /**
>>> + * iio_channel_cb_set_buffer_size() - set the buffer length.
>>> + * @cb_buffer: The callback buffer from whom we want the channel
>>> + * information.
>>> + * @length: buffer length in bytes
>>> + * @watermark: buffer watermark in bytes
>>> + *
>>> + * This function allows to configure the buffer length. The watermark if
>>> + * forced to half of the buffer.
>>> + */
>>> +int iio_channel_cb_set_buffer_params(struct iio_cb_buffer *cb_buffer,
>>> + size_t length, size_t watermark);
>>> +
>>> +/**
>>> * iio_channel_release_all_cb() - release and unregister the callback.
>>> * @cb_buffer: The callback buffer that was allocated.
>>> */
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v3] iio:light: Add support for STMicro VL6180 sensor
From: Jonathan Cameron @ 2017-03-25 16:21 UTC (permalink / raw)
To: Manivannan Sadhasivam, pmeerw-jW+XmwGofnusTnJN9+BGXg
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170319145312.GA10949-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 19/03/17 14:53, Manivannan Sadhasivam wrote:
> This patch adds support for STMicro VL6180 - ALS, range and proximity sensor. Sensor is capable of measuring the light
> intensity as well as object distance using TOF (Time of Flight) technology.
>
> Signed-off-by: Manivannan Sadhasivam <manivannanece23-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Peter Meerwald-Stadler <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
There were a couple of minor formatting bits and bobs that checkpatch
threw up so I fixed those whilst applying.
Left a few overly long lines, so we may well get some patches from others
for those.
Thanks,
Jonathan
> ---
>
> Changes in v3:
>
> 1. Changed the false reset handling. Instead of error out, logging is adapted
>
> Changes in v2:
>
> 1. Specified range in comments and Kconfig
> 2. Used cpu_to_be16 for endianess conversion
> 3. Fixed typos
> 4. Added necessary comments
> 5. Added Peter's signed-off-by:
>
> .../devicetree/bindings/iio/light/vl6180.txt | 16 +
> drivers/iio/light/Kconfig | 10 +
> drivers/iio/light/Makefile | 1 +
> drivers/iio/light/vl6180.c | 538 +++++++++++++++++++++
> 4 files changed, 565 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/light/vl6180.txt
> create mode 100644 drivers/iio/light/vl6180.c
>
> diff --git a/Documentation/devicetree/bindings/iio/light/vl6180.txt b/Documentation/devicetree/bindings/iio/light/vl6180.txt
> new file mode 100644
> index 0000000..8f77ccd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/vl6180.txt
> @@ -0,0 +1,16 @@
> +STMicro VL6180 - ALS, range and proximity sensor
> +
> +Link to datasheet: http://www.st.com/resource/en/datasheet/vl6180x.pdf
> +
> +Required properties:
> +
> + -compatible: should be "st,vl6180"
> + -reg: the I2C address of the sensor
> +
> +Example:
> +
> +vl6180@29 {
> + compatible = "st,vl6180";
> + reg = <0x29>;
> +};
> +
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 7c566f5..aec04ef 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -352,4 +352,14 @@ config VEML6070
> To compile this driver as a module, choose M here: the
> module will be called veml6070.
>
> +config VL6180
> + tristate "VL6180 ALS, range and proximity sensor"
> + depends on I2C
> + help
> + Say Y here if you want to build a driver for the STMicroelectronics
> + VL6180 combined ambient light, range and proximity sensor.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called vl6180.
> +
> endmenu
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 6f2a3c6..24a8d72 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -33,3 +33,4 @@ obj-$(CONFIG_TSL4531) += tsl4531.o
> obj-$(CONFIG_US5182D) += us5182d.o
> obj-$(CONFIG_VCNL4000) += vcnl4000.o
> obj-$(CONFIG_VEML6070) += veml6070.o
> +obj-$(CONFIG_VL6180) += vl6180.o
> diff --git a/drivers/iio/light/vl6180.c b/drivers/iio/light/vl6180.c
> new file mode 100644
> index 0000000..6bfdca3
> --- /dev/null
> +++ b/drivers/iio/light/vl6180.c
> @@ -0,0 +1,538 @@
> +/*
> + * vl6180.c - Support for STMicroelectronics VL6180 ALS, range and proximity sensor
> + *
> + * Copyright 2017 Peter Meerwald-Stadler <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>
> + * Copyright 2017 Manivannan Sadhasivam <manivannanece23-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * IIO driver for VL6180 (7-bit I2C slave address 0x29)
> + *
> + * Range: 0 to 100mm
> + * ALS: < 1 Lux up to 100 kLux
> + * IR: 850nm
> + *
> + * TODO: irq, threshold events, continuous mode, hardware buffer
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/delay.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define VL6180_DRV_NAME "vl6180"
> +
> +/* Device identification register and value */
> +#define VL6180_MODEL_ID 0x000
> +#define VL6180_MODEL_ID_VAL 0xb4
> +
> +/* Configuration registers */
> +#define VL6180_INTR_CONFIG 0x014
> +#define VL6180_INTR_CLEAR 0x015
> +#define VL6180_OUT_OF_RESET 0x016
> +#define VL6180_HOLD 0x017
> +#define VL6180_RANGE_START 0x018
> +#define VL6180_ALS_START 0x038
> +#define VL6180_ALS_GAIN 0x03f
> +#define VL6180_ALS_IT 0x040
> +
> +/* Status registers */
> +#define VL6180_RANGE_STATUS 0x04d
> +#define VL6180_ALS_STATUS 0x04e
> +#define VL6180_INTR_STATUS 0x04f
> +
> +/* Result value registers */
> +#define VL6180_ALS_VALUE 0x050
> +#define VL6180_RANGE_VALUE 0x062
> +#define VL6180_RANGE_RATE 0x066
> +
> +/* bits of the RANGE_START and ALS_START register */
> +#define VL6180_MODE_CONT BIT(1) /* continuous mode */
> +#define VL6180_STARTSTOP BIT(0) /* start measurement, auto-reset */
> +
> +/* bits of the INTR_STATUS and INTR_CONFIG register */
> +#define VL6180_ALS_READY BIT(5)
> +#define VL6180_RANGE_READY BIT(2)
> +
> +/* bits of the INTR_CLEAR register */
> +#define VL6180_CLEAR_ERROR BIT(2)
> +#define VL6180_CLEAR_ALS BIT(1)
> +#define VL6180_CLEAR_RANGE BIT(0)
> +
> +/* bits of the HOLD register */
> +#define VL6180_HOLD_ON BIT(0)
> +
> +/* default value for the ALS_IT register */
> +#define VL6180_ALS_IT_100 0x63 /* 100 ms */
> +
> +/* values for the ALS_GAIN register */
> +#define VL6180_ALS_GAIN_1 0x46
> +#define VL6180_ALS_GAIN_1_25 0x45
> +#define VL6180_ALS_GAIN_1_67 0x44
> +#define VL6180_ALS_GAIN_2_5 0x43
> +#define VL6180_ALS_GAIN_5 0x42
> +#define VL6180_ALS_GAIN_10 0x41
> +#define VL6180_ALS_GAIN_20 0x40
> +#define VL6180_ALS_GAIN_40 0x47
> +
> +struct vl6180_data {
> + struct i2c_client *client;
> + struct mutex lock;
> +};
> +
> +enum { VL6180_ALS, VL6180_RANGE, VL6180_PROX };
> +
> +/**
> + * struct vl6180_chan_regs - Registers for accessing channels
> + * @drdy_mask: Data ready bit in status register
> + * @start_reg: Conversion start register
> + * @value_reg: Result value register
> + * @word: Register word length
> + */
> +struct vl6180_chan_regs {
> + u8 drdy_mask;
> + u16 start_reg, value_reg;
> + bool word;
> +};
> +
> +static const struct vl6180_chan_regs vl6180_chan_regs_table[] = {
> + [VL6180_ALS] = {
> + .drdy_mask = VL6180_ALS_READY,
> + .start_reg = VL6180_ALS_START,
> + .value_reg = VL6180_ALS_VALUE,
> + .word = true,
> + },
> + [VL6180_RANGE] = {
> + .drdy_mask = VL6180_RANGE_READY,
> + .start_reg = VL6180_RANGE_START,
> + .value_reg = VL6180_RANGE_VALUE,
> + .word = false,
> + },
> + [VL6180_PROX] = {
> + .drdy_mask = VL6180_RANGE_READY,
> + .start_reg = VL6180_RANGE_START,
> + .value_reg = VL6180_RANGE_RATE,
> + .word = true,
> + },
> +};
> +
> +static int vl6180_read(struct i2c_client *client, u16 cmd, void *databuf,
> + u8 len)
> +{
> + __be16 cmdbuf = cpu_to_be16(cmd);
> + struct i2c_msg msgs[2] = {
> + { .addr = client->addr, .len = sizeof(cmdbuf), .buf = (u8 *) &cmdbuf },
> + { .addr = client->addr, .len = len, .buf = databuf,
> + .flags = I2C_M_RD } };
> + int ret;
> +
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret < 0)
> + dev_err(&client->dev, "failed reading register 0x%04x\n", cmd);
> +
> + return ret;
> +}
> +
> +static int vl6180_read_byte(struct i2c_client *client, u16 cmd)
> +{
> + u8 data;
> + int ret;
> +
> + ret = vl6180_read(client, cmd, &data, sizeof(data));
> + if (ret < 0)
> + return ret;
> +
> + return data;
> +}
> +
> +static int vl6180_read_word(struct i2c_client *client, u16 cmd)
> +{
> + __be16 data;
> + int ret;
> +
> + ret = vl6180_read(client, cmd, &data, sizeof(data));
> + if (ret < 0)
> + return ret;
> +
> + return be16_to_cpu(data);
> +}
> +
> +static int vl6180_write_byte(struct i2c_client *client, u16 cmd, u8 val)
> +{
> + u8 buf[3];
> + struct i2c_msg msgs[1] = {
> + { .addr = client->addr, .len = sizeof(buf), .buf = (u8 *) &buf } };
> + int ret;
> +
> + buf[0] = cmd >> 8;
> + buf[1] = cmd & 0xff;
> + buf[2] = val;
> +
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret < 0) {
> + dev_err(&client->dev, "failed writing register 0x%04x\n", cmd);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int vl6180_write_word(struct i2c_client *client, u16 cmd, u16 val)
> +{
> + __be16 buf[2];
> + struct i2c_msg msgs[1] = {
> + { .addr = client->addr, .len = sizeof(buf), .buf = (u8 *) &buf } };
> + int ret;
> +
> + buf[0] = cpu_to_be16(cmd);
> + buf[1] = cpu_to_be16(val);
> +
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret < 0) {
> + dev_err(&client->dev, "failed writing register 0x%04x\n", cmd);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int vl6180_measure(struct vl6180_data *data, int addr)
> +{
> + struct i2c_client *client = data->client;
> + int tries = 20, ret;
> + u16 value;
> +
> + mutex_lock(&data->lock);
> + /* Start single shot measurement */
> + ret = vl6180_write_byte(client,
> + vl6180_chan_regs_table[addr].start_reg, VL6180_STARTSTOP);
> + if (ret < 0)
> + goto fail;
> +
> + while (tries--) {
> + ret = vl6180_read_byte(client, VL6180_INTR_STATUS);
> + if (ret < 0)
> + goto fail;
> +
> + if (ret & vl6180_chan_regs_table[addr].drdy_mask)
> + break;
> + msleep(20);
> + }
> +
> + if (tries < 0) {
> + ret = -EIO;
> + goto fail;
> + }
> +
> + /* Read result value from appropriate registers */
> + ret = vl6180_chan_regs_table[addr].word ?
> + vl6180_read_word(client, vl6180_chan_regs_table[addr].value_reg) :
> + vl6180_read_byte(client, vl6180_chan_regs_table[addr].value_reg);
> + if (ret < 0)
> + goto fail;
> + value = ret;
> +
> + /* Clear the interrupt flag after data read */
> + ret = vl6180_write_byte(client, VL6180_INTR_CLEAR,
> + VL6180_CLEAR_ERROR | VL6180_CLEAR_ALS | VL6180_CLEAR_RANGE);
> + if (ret < 0)
> + goto fail;
> +
> + ret = value;
> +
> +fail:
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static const struct iio_chan_spec vl6180_channels[] = {
> + {
> + .type = IIO_LIGHT,
> + .address = VL6180_ALS,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_INT_TIME) |
> + BIT(IIO_CHAN_INFO_SCALE) |
> + BIT(IIO_CHAN_INFO_HARDWAREGAIN),
> + }, {
> + .type = IIO_DISTANCE,
> + .address = VL6180_RANGE,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + }, {
> + .type = IIO_PROXIMITY,
> + .address = VL6180_PROX,
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> + }
> +};
> +
> +/* Columns 3 & 4 represent the same value in decimal and hex notations.
> + * Kept in order to avoid the datatype conversion while reading the
> + * hardware_gain */
> +static const int vl6180_als_gain[8][4] = {
> + { 1, 0, 70, VL6180_ALS_GAIN_1 },
> + { 1, 250000, 69, VL6180_ALS_GAIN_1_25 },
> + { 1, 670000, 68, VL6180_ALS_GAIN_1_67 },
> + { 2, 500000, 67, VL6180_ALS_GAIN_2_5 },
> + { 5, 0, 66, VL6180_ALS_GAIN_5 },
> + { 10, 0, 65, VL6180_ALS_GAIN_10 },
> + { 20, 0, 64, VL6180_ALS_GAIN_20 },
> + { 40, 0, 71, VL6180_ALS_GAIN_40 }
> +};
> +
> +static int vl6180_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct vl6180_data *data = iio_priv(indio_dev);
> + int ret, i;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = vl6180_measure(data, chan->address);
> + if (ret < 0)
> + return ret;
> + *val = ret;
> +
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_INT_TIME:
> + ret = vl6180_read_word(data->client, VL6180_ALS_IT);
> + if (ret < 0)
> + return ret;
> + *val = 0; /* 1 count = 1ms (0 = 1ms) */
> + *val2 = (ret + 1) * 1000; /* convert to seconds */
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_SCALE:
> + switch (chan->type) {
> + case IIO_LIGHT:
> + *val = 0; /* one ALS count is 0.32 Lux */
> + *val2 = 320000;
> + break;
> + case IIO_DISTANCE:
> + *val = 0; /* sensor reports mm, scale to meter */
> + *val2 = 1000;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_CHAN_INFO_HARDWAREGAIN:
> + ret = vl6180_read_byte(data->client, VL6180_ALS_GAIN);
> + if (ret < 0)
> + return -EINVAL;
> + for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
> + if (ret == vl6180_als_gain[i][2]) {
> + *val = vl6180_als_gain[i][0];
> + *val2 = vl6180_als_gain[i][1];
> + }
> + }
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static IIO_CONST_ATTR(als_gain_available, "1 1.25 1.67 2.5 5 10 20 40");
> +
> +static struct attribute *vl6180_attributes[] = {
> + &iio_const_attr_als_gain_available.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group vl6180_attribute_group = {
> + .attrs = vl6180_attributes,
> +};
> +
> +/* HOLD is needed before updating any config registers */
> +static int vl6180_hold(struct vl6180_data *data, bool hold)
> +{
> + return vl6180_write_byte(data->client, VL6180_HOLD,
> + hold ? VL6180_HOLD_ON : 0);
> +}
> +
> +static int vl6180_set_als_gain(struct vl6180_data *data, int val, int val2)
> +{
> + int i, ret;
> +
> + for (i = 0; i < ARRAY_SIZE(vl6180_als_gain); i++) {
> + if (val == vl6180_als_gain[i][0] &&
> + val2 == vl6180_als_gain[i][1]) {
> + mutex_lock(&data->lock);
> + ret = vl6180_hold(data, true);
> + if (ret < 0)
> + goto fail;
> + ret = vl6180_write_byte(data->client, VL6180_ALS_GAIN,
> + vl6180_als_gain[i][3]);
> +fail:
> + vl6180_hold(data, false);
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int vl6180_set_it(struct vl6180_data *data, int val2)
> +{
> + int ret;
> +
> + mutex_lock(&data->lock);
> + ret = vl6180_hold(data, true);
> + if (ret < 0)
> + goto fail;
> + ret = vl6180_write_word(data->client, VL6180_ALS_IT,
> + (val2 - 500) / 1000); /* write value in ms */
> +fail:
> + vl6180_hold(data, false);
> + mutex_unlock(&data->lock);
> +
> + return ret;
> +}
> +
> +static int vl6180_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct vl6180_data *data = iio_priv(indio_dev);
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_INT_TIME:
> + if (val != 0 || val2 < 500 || val2 >= 512500)
> + return -EINVAL;
> +
> + return vl6180_set_it(data, val2);
> + case IIO_CHAN_INFO_HARDWAREGAIN:
> + if (chan->type != IIO_LIGHT)
> + return -EINVAL;
> +
> + return vl6180_set_als_gain(data, val, val2);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static const struct iio_info vl6180_info = {
> + .read_raw = vl6180_read_raw,
> + .write_raw = vl6180_write_raw,
> + .attrs = &vl6180_attribute_group,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static int vl6180_init(struct vl6180_data *data)
> +{
> + struct i2c_client *client = data->client;
> + int ret;
> +
> + ret = vl6180_read_byte(client, VL6180_MODEL_ID);
> + if (ret < 0)
> + return ret;
> +
> + if (ret != VL6180_MODEL_ID_VAL) {
> + dev_err(&client->dev, "invalid model ID %02x\n", ret);
> + return -ENODEV;
> + }
> +
> + ret = vl6180_hold(data, true);
> + if (ret < 0)
> + return ret;
> +
> + ret = vl6180_read_byte(client, VL6180_OUT_OF_RESET);
> + if (ret < 0)
> + return ret;
> +
> + /* Detect false reset condition here. This bit is always set when the system comes
> + * out of reset */
> + if (ret != 0x01)
> + dev_info(&client->dev, "device is not fresh out of reset\n");
> +
> + /* Enable ALS and Range ready interrupts */
> + ret = vl6180_write_byte(client, VL6180_INTR_CONFIG,
> + VL6180_ALS_READY | VL6180_RANGE_READY);
> + if (ret < 0)
> + return ret;
> +
> + /* ALS integration time: 100ms */
> + ret = vl6180_write_word(client, VL6180_ALS_IT, VL6180_ALS_IT_100);
> + if (ret < 0)
> + return ret;
> +
> + /* ALS gain: 1 */
> + ret = vl6180_write_byte(client, VL6180_ALS_GAIN, VL6180_ALS_GAIN_1);
> + if (ret < 0)
> + return ret;
> +
> + ret = vl6180_write_byte(client, VL6180_OUT_OF_RESET, 0x00);
> + if (ret < 0)
> + return ret;
> +
> + return vl6180_hold(data, false);
> +}
> +
> +static int vl6180_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct vl6180_data *data;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + i2c_set_clientdata(client, indio_dev);
> + data->client = client;
> + mutex_init(&data->lock);
> +
> + indio_dev->dev.parent = &client->dev;
> + indio_dev->info = &vl6180_info;
> + indio_dev->channels = vl6180_channels;
> + indio_dev->num_channels = ARRAY_SIZE(vl6180_channels);
> + indio_dev->name = VL6180_DRV_NAME;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = vl6180_init(data);
> + if (ret < 0)
> + return ret;
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct of_device_id vl6180_of_match[] = {
> + { .compatible = "st,vl6180", },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, vl6180_of_match);
> +
> +static const struct i2c_device_id vl6180_id[] = {
> + { "vl6180", 0 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, vl6180_id);
> +
> +static struct i2c_driver vl6180_driver = {
> + .driver = {
> + .name = VL6180_DRV_NAME,
> + .of_match_table = of_match_ptr(vl6180_of_match),
> + },
> + .probe = vl6180_probe,
> + .id_table = vl6180_id,
> +};
> +
> +module_i2c_driver(vl6180_driver);
> +
> +MODULE_AUTHOR("Peter Meerwald-Stadler <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>");
> +MODULE_AUTHOR("Manivannan Sadhasivam <manivannanece23-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicro VL6180 ALS, range and proximity sensor driver");
> +MODULE_LICENSE("GPL");
>
--
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 0/2] Meson8b support for the meson_saradc driver
From: Martin Blumenstingl @ 2017-03-25 16:29 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Martin Blumenstingl
The SAR ADC register layout seems to be mostly the same on older SoCs.
Thus basically all functionality is already supported by the existing
driver.
There are two small differences though:
- the adc_clk and adc_div clock are not provided by the clock-controller
on Meson8b. instead the SAR ADC provides an internal "adc_clk" (this
behavior is already supported by the driver and requires no changes)
- the newer SoCs are using some register bits only the kernel or the
BL30 (bootloader) are using the SAR ADC. This is the main change of
this series: guarding all BL30 specific code with a corresponding
"if"-block.
This also adds a new DT binding for the SAR ADC in Meson8b because the
driver has to specify (for this older version) that there's no BL30
integration available (and these register bits should not be touched).
Martin Blumenstingl (2):
Documentation: dt-bindings: add a Meson8b compatible to the SAR ADC
iio: adc: meson-saradc: add Meson8b SoC compatibility
.../bindings/iio/adc/amlogic,meson-saradc.txt | 1 +
drivers/iio/adc/meson_saradc.c | 70 ++++++++++++++--------
2 files changed, 47 insertions(+), 24 deletions(-)
--
2.12.1
^ permalink raw reply
* [PATCH 1/2] Documentation: dt-bindings: add a Meson8b compatible to the SAR ADC
From: Martin Blumenstingl @ 2017-03-25 16:29 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Martin Blumenstingl
In-Reply-To: <20170325162938.28659-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
The Amlogic Meson SAR ADC driver can be used on Meson8b as well
(probably on earlier SoC generations as well, but I don't have any
hardware available for testing that).
Add a separate compatible for Meson8b because it does not need any of
the BL30 magic (unlike the GX SoCs).
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
index f9e3ff2c656e..84a931a0ff1f 100644
--- a/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/amlogic,meson-saradc.txt
@@ -2,6 +2,7 @@
Required properties:
- compatible: depending on the SoC this should be one of:
+ - "amlogic,meson8b-saradc" for Meson8b
- "amlogic,meson-gxbb-saradc" for GXBB
- "amlogic,meson-gxl-saradc" for GXL
- "amlogic,meson-gxm-saradc" for GXM
--
2.12.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/2] iio: adc: meson-saradc: add Meson8b SoC compatibility
From: Martin Blumenstingl @ 2017-03-25 16:29 UTC (permalink / raw)
To: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: carlo-KA+7E9HrN00dnm+yROfE0A, khilman-rdvid1DuHRBWk0Htik3J/w,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Martin Blumenstingl
In-Reply-To: <20170325162938.28659-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Meson GX SoCs however use some magic bits to prevent simultaneous (=
conflicting, because only consumer should use the FIFO buffer with the
ADC results) usage by the Linux kernel and the bootloader (the BL30
bootloader uses the SAR ADC to read the CPU temperature).
This patch changes guards all BL30 functionality so it is skipped on
SoCs which don't have it. Since the hardware itself doesn't know whether
BL30 is available the internal meson_sar_adc_data is extended so this
information can be provided per of_device_id.data inside the driver.
Additionally the clocks "adc_clk" and "adc_sel" are not provided by the
clock-controller itself. "adc_sel" is not available at all. "adc_clk"
is provided by the SAR ADC IP block itself on Meson8b (and earlier).
This is already supported by the meson_saradc driver.
Finally a new of_device_id for the Meson8b SoC is added so it can be
wired up in the corresponding DT.
Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
drivers/iio/adc/meson_saradc.c | 70 +++++++++++++++++++++++++++---------------
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
index dd4190b50df6..f78fe66e0ec8 100644
--- a/drivers/iio/adc/meson_saradc.c
+++ b/drivers/iio/adc/meson_saradc.c
@@ -220,6 +220,7 @@ enum meson_sar_adc_chan7_mux_sel {
};
struct meson_sar_adc_data {
+ char has_bl30_integration;
unsigned int resolution;
const char *name;
};
@@ -437,19 +438,24 @@ static int meson_sar_adc_lock(struct iio_dev *indio_dev)
mutex_lock(&indio_dev->mlock);
- /* prevent BL30 from using the SAR ADC while we are using it */
- regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
- MESON_SAR_ADC_DELAY_KERNEL_BUSY,
- MESON_SAR_ADC_DELAY_KERNEL_BUSY);
-
- /* wait until BL30 releases it's lock (so we can use the SAR ADC) */
- do {
- udelay(1);
- regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
- } while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
-
- if (timeout < 0)
- return -ETIMEDOUT;
+ if (priv->data->has_bl30_integration) {
+ /* prevent BL30 from using the SAR ADC while we are using it */
+ regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
+ MESON_SAR_ADC_DELAY_KERNEL_BUSY,
+ MESON_SAR_ADC_DELAY_KERNEL_BUSY);
+
+ /*
+ * wait until BL30 releases it's lock (so we can use the SAR
+ * ADC)
+ */
+ do {
+ udelay(1);
+ regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
+ } while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
+
+ if (timeout < 0)
+ return -ETIMEDOUT;
+ }
return 0;
}
@@ -458,9 +464,10 @@ static void meson_sar_adc_unlock(struct iio_dev *indio_dev)
{
struct meson_sar_adc_priv *priv = iio_priv(indio_dev);
- /* allow BL30 to use the SAR ADC again */
- regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
- MESON_SAR_ADC_DELAY_KERNEL_BUSY, 0);
+ if (priv->data->has_bl30_integration)
+ /* allow BL30 to use the SAR ADC again */
+ regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
+ MESON_SAR_ADC_DELAY_KERNEL_BUSY, 0);
mutex_unlock(&indio_dev->mlock);
}
@@ -614,14 +621,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
*/
meson_sar_adc_set_chan7_mux(indio_dev, CHAN7_MUX_CH7_INPUT);
- /*
- * leave sampling delay and the input clocks as configured by BL30 to
- * make sure BL30 gets the values it expects when reading the
- * temperature sensor.
- */
- regmap_read(priv->regmap, MESON_SAR_ADC_REG3, ®val);
- if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
- return 0;
+ if (priv->data->has_bl30_integration) {
+ /*
+ * leave sampling delay and the input clocks as configured by
+ * BL30 to make sure BL30 gets the values it expects when
+ * reading the temperature sensor.
+ */
+ regmap_read(priv->regmap, MESON_SAR_ADC_REG3, ®val);
+ if (regval & MESON_SAR_ADC_REG3_BL30_INITIALIZED)
+ return 0;
+ }
meson_sar_adc_stop_sample_engine(indio_dev);
@@ -834,23 +843,36 @@ static const struct iio_info meson_sar_adc_iio_info = {
.driver_module = THIS_MODULE,
};
+struct meson_sar_adc_data meson_sar_adc_meson8b_data = {
+ .has_bl30_integration = 0,
+ .resolution = 10,
+ .name = "meson-meson8b-saradc",
+};
+
struct meson_sar_adc_data meson_sar_adc_gxbb_data = {
+ .has_bl30_integration = 1,
.resolution = 10,
.name = "meson-gxbb-saradc",
};
struct meson_sar_adc_data meson_sar_adc_gxl_data = {
+ .has_bl30_integration = 1,
.resolution = 12,
.name = "meson-gxl-saradc",
};
struct meson_sar_adc_data meson_sar_adc_gxm_data = {
+ .has_bl30_integration = 1,
.resolution = 12,
.name = "meson-gxm-saradc",
};
static const struct of_device_id meson_sar_adc_of_match[] = {
{
+ .compatible = "amlogic,meson8b-saradc",
+ .data = &meson_sar_adc_meson8b_data,
+ },
+ {
.compatible = "amlogic,meson-gxbb-saradc",
.data = &meson_sar_adc_gxbb_data,
}, {
--
2.12.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox