All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [GIT PULL] Renesas ARM64 Based SoC Defconfig Updates for v4.10
From: Olof Johansson @ 2016-11-08  3:16 UTC (permalink / raw)
  To: Simon Horman
  Cc: arm, linux-renesas-soc, Kevin Hilman, Arnd Bergmann,
	linux-arm-kernel, Magnus Damm
In-Reply-To: <cover.1478078567.git.horms+renesas@verge.net.au>

On Wed, Nov 02, 2016 at 10:23:40AM +0100, Simon Horman wrote:
> Hi Olof, Hi Kevin, Hi Arnd,
> 
> Please consider these Renesas ARM64 based SoC defconfig updates for v4.10.
> 
> 
> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> 
>   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas.git tags/renesas-arm64-defconfig-for-v4.10
> 
> for you to fetch changes up to 833c97254724a620d1d9fb9a580681fb52536fbf:
> 
>   arm64: defconfig: Enable DRM DU and V4L2 FCP + VSP modules (2016-10-27 09:07:35 +0200)

Merged, thanks.


-Olof

^ permalink raw reply

* Re: [PATCH 6/7] pci: Remove the irq_affinity mask from struct pci_dev
From: Bjorn Helgaas @ 2016-11-08 20:59 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: tglx, axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478544462-9549-7-git-send-email-hch@lst.de>

s/pci/PCI/ (in subject)

On Mon, Nov 07, 2016 at 10:47:41AM -0800, Christoph Hellwig wrote:
> This has never been used, and now is totally unreferenced.  Nuke it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  include/linux/pci.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 7090f5f..f2ba6ac 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -333,7 +333,6 @@ struct pci_dev {
>  	 * directly, use the values stored here. They might be different!
>  	 */
>  	unsigned int	irq;
> -	struct cpumask	*irq_affinity;
>  	struct resource resource[DEVICE_COUNT_RESOURCE]; /* I/O and memory regions + expansion ROMs */
>  
>  	bool match_driver;		/* Skip attaching driver */
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" 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 v5 2/5] driver core: Functional dependencies tracking support
From: Luis R. Rodriguez @ 2016-11-08 20:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Geert Uytterhoeven, Andrzej Hajda
  Cc: Luis R. Rodriguez, Lukas Wunner, Rafael J. Wysocki, Linux PM list,
	Alan Stern, Linux Kernel Mailing List, Tomeu Vizoso, Mark Brown,
	Marek Szyprowski, Kevin Hilman, Ulf Hansson, Laurent Pinchart,
	Lars-Peter Clausen, Grant Likely, Mauro Carvalho Chehab,
	Andrzej Hajda, Dmitry Torokhov
In-Reply-To: <20161108194335.GA22680@kroah.com>

On Tue, Nov 08, 2016 at 08:43:35PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Nov 08, 2016 at 08:21:04PM +0100, Luis R. Rodriguez wrote:
> > On Tue, Nov 08, 2016 at 07:45:41AM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Nov 07, 2016 at 10:22:50PM +0100, Luis R. Rodriguez wrote:
> > > > We have no explicit semantics to check if a driver / subsystem
> > > > supports deferred probe.
> > > 
> > > Why would we need such a thing?
> > 
> > It depends on the impact of a driver/subsystem not properly supporting
> > deffered probe, if this is no-op then such a need is not critical but
> > would be good to proactively inform developers / users so they avoid 
> > its use, if this will cause issues its perhaps best to make this a
> > no-op through a check. AFAICT reviewing implications of not supporting
> > deferred probe on drivers/subsytsems for this framework is not clearly
> > spelled out, if we start considering re-using this framework for probe
> > ordering I'd hate to see issues come up without this corner case being
> > concretely considered.
> 
> It should not matter to the driver core if a subsystem, or a driver,
> supports or does not support deferred probe.

That was my impression as well -- however Rafael noted this as an area
worth highlighting. So perhaps he can elaborate.

But at least as per my notes I do have here Geert Uytterhoeven reminding
us that:

  "Some drivers / subsystems don’t support deferred probe yet, such failures
   usually don’t blow up, but cause subtle malfunctioning. Example, an Ethernet
   phy could not get its interrupt as the primary IRQ chip had not been probed
   yet, it reverted to polling though. Sub-optimal." [0]

[0] https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2016-August/003425.html

This sounds more like the existing deffered probe solution has detrimental
unexpected effects on some drivers / subsystems -- its not clear *why*, but
its worth reviewing if there are other drivers and seeing if we need this
annotation.

> It's a quick and simple solution to a complex problem that works well.

We all agree. The recent discussions over probe ordering are simply
optimization considerations -- should the time incurred to use deferred
probe be X and the time incurred when using an alternative is Y and we
determine the time Y is < X we have a winning alternative. Part of my
own big issue with deferred probe is a) its extremely non-deterministic,
b) it seems some folks have thought about similar problems and we might
really be able to do better. I'm not convinced that the functional
dependencies patches are the panacea for probe ordering, and while it
was not intended for that, some folks are assuming it could be. To really
vet this prospect we must really consider what other subsystem have done
and review other alternatives efforts.

> Yes, you can iterate a
> lot of times, but that's fine, we have time at boot to do that (and
> really, it is fast.)

Deferred probe is left for late_initcall() [1] -- this *assumes* that the
driver/subsystem can be loaded so late, and as per Andrzej this solution
isunacceptable/undesirable. So it would be unfair and incorrect to categorize
all drivers in the same boat, in fact given this lone fact it may be
worth revisiting the idea I mentioned about a capability aspect to support
a late deferred probe later. We tend to assume its fine, however it does
not seem to be the case.

[1] drivers/base/dd.c:late_initcall(deferred_probe_initcall);

> > Furthermore -- how does this framework compare to Andrzej's resource tracking
> > solution? I confess I have not had a chance yet to review yet but in light of
> > this question it would be good to know if Andrzej's framework also requires
> > deferred probe as similar concerns would exist there as well.
> 
> I have no idea what "framework" you are talking about here, do you have
> a pointer to patches?

I'm surprised given Andrzej did both Cc you on his patches [2] *and* chimed
in on Rafael's patches to indicate that we likely can integrate PM concerns
into his own "framework" [3]. There was no resolution to this discussion, however
its not IMHO sufficient to brush off Andrzej's points in particular because
Andrzej *is* indicating that his framework:

- Eliminates deferred probe and resulting late_initcall(), consumer registers
callbacks informing when given resources (clock, regulator, etc) becomes
available
- Properly handle resource disappearance (driver unbind, hotplug)
- Track resources which are not vital to the device, but can influence behavior
- Offers simplified resource allocation
- Can be easily expanded to help with power management

Granted I have not reviewed this yet but it at least was on my radar, and
I do believe its worth reviewing this further given the generally expressed
interest to see if we can have a common framework to address both ordering
problems, suspend and probe. At a quick glance the "ghost provider" idea
seems like a rather crazy idea but hey, there may be some goods in there.

It was sad both Andrzej and yourself could not attend the complex dependencies
tracks -- I think it would have been useful. I don't expect us to address a
resolution to probe ordering immediately -- but I am in the hopes we at least
can keep an open mind about the similarity of the problems and see if we can
aim for a clean elegant solution that might help both.

[2] https://lwn.net/Articles/625454/
[3] http://thread.gmane.org/gmane.linux.kernel/2087152

  Luis

^ permalink raw reply

* Re: [PATCH v5 2/5] driver core: Functional dependencies tracking support
From: Luis R. Rodriguez @ 2016-11-08 20:58 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Geert Uytterhoeven
  Cc: Luis R. Rodriguez, Lukas Wunner, Rafael J. Wysocki, Linux PM list,
	Alan Stern, Linux Kernel Mailing List, Tomeu Vizoso, Mark Brown,
	Marek Szyprowski, Kevin Hilman, Ulf Hansson, Laurent Pinchart,
	Lars-Peter Clausen, Grant Likely, Mauro Carvalho Chehab,
	Andrzej Hajda, Dmitry Torokhov
In-Reply-To: <20161108194335.GA22680@kroah.com>

On Tue, Nov 08, 2016 at 08:43:35PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Nov 08, 2016 at 08:21:04PM +0100, Luis R. Rodriguez wrote:
> > On Tue, Nov 08, 2016 at 07:45:41AM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Nov 07, 2016 at 10:22:50PM +0100, Luis R. Rodriguez wrote:
> > > > We have no explicit semantics to check if a driver / subsystem
> > > > supports deferred probe.
> > > 
> > > Why would we need such a thing?
> > 
> > It depends on the impact of a driver/subsystem not properly supporting
> > deffered probe, if this is no-op then such a need is not critical but
> > would be good to proactively inform developers / users so they avoid 
> > its use, if this will cause issues its perhaps best to make this a
> > no-op through a check. AFAICT reviewing implications of not supporting
> > deferred probe on drivers/subsytsems for this framework is not clearly
> > spelled out, if we start considering re-using this framework for probe
> > ordering I'd hate to see issues come up without this corner case being
> > concretely considered.
> 
> It should not matter to the driver core if a subsystem, or a driver,
> supports or does not support deferred probe.

That was my impression as well -- however Rafael noted this as an area
worth highlighting. So perhaps he can elaborate.

But at least as per my notes I do have here Geert Uytterhoeven reminding
us that:

  "Some drivers / subsystems don’t support deferred probe yet, such failures
   usually don’t blow up, but cause subtle malfunctioning. Example, an Ethernet
   phy could not get its interrupt as the primary IRQ chip had not been probed
   yet, it reverted to polling though. Sub-optimal." [0]

[0] https://lists.linuxfoundation.org/pipermail/ksummit-discuss/2016-August/003425.html

This sounds more like the existing deffered probe solution has detrimental
unexpected effects on some drivers / subsystems -- its not clear *why*, but
its worth reviewing if there are other drivers and seeing if we need this
annotation.

> It's a quick and simple solution to a complex problem that works well.

We all agree. The recent discussions over probe ordering are simply
optimization considerations -- should the time incurred to use deferred
probe be X and the time incurred when using an alternative is Y and we
determine the time Y is < X we have a winning alternative. Part of my
own big issue with deferred probe is a) its extremely non-deterministic,
b) it seems some folks have thought about similar problems and we might
really be able to do better. I'm not convinced that the functional
dependencies patches are the panacea for probe ordering, and while it
was not intended for that, some folks are assuming it could be. To really
vet this prospect we must really consider what other subsystem have done
and review other alternatives efforts.

> Yes, you can iterate a
> lot of times, but that's fine, we have time at boot to do that (and
> really, it is fast.)

Deferred probe is left for late_initcall() [1] -- this *assumes* that the
driver/subsystem can be loaded so late, and as per Andrzej this solution
isunacceptable/undesirable. So it would be unfair and incorrect to categorize
all drivers in the same boat, in fact given this lone fact it may be
worth revisiting the idea I mentioned about a capability aspect to support
a late deferred probe later. We tend to assume its fine, however it does
not seem to be the case.

[1] drivers/base/dd.c:late_initcall(deferred_probe_initcall);

> > Furthermore -- how does this framework compare to Andrzej's resource tracking
> > solution? I confess I have not had a chance yet to review yet but in light of
> > this question it would be good to know if Andrzej's framework also requires
> > deferred probe as similar concerns would exist there as well.
> 
> I have no idea what "framework" you are talking about here, do you have
> a pointer to patches?

I'm surprised given Andrzej did both Cc you on his patches [2] *and* chimed
in on Rafael's patches to indicate that we likely can integrate PM concerns
into his own "framework" [3]. There was no resolution to this discussion, however
its not IMHO sufficient to brush off Andrzej's points in particular because
Andrzej *is* indicating that his framework:

- Eliminates deferred probe and resulting late_initcall(), consumer registers
callbacks informing when given resources (clock, regulator, etc) becomes
available
- Properly handle resource disappearance (driver unbind, hotplug)
- Track resources which are not vital to the device, but can influence behavior
- Offers simplified resource allocation
- Can be easily expanded to help with power management

Granted I have not reviewed this yet but it at least was on my radar, and
I do believe its worth reviewing this further given the generally expressed
interest to see if we can have a common framework to address both ordering
problems, suspend and probe. At a quick glance the "ghost provider" idea
seems like a rather crazy idea but hey, there may be some goods in there.

It was sad both Andrzej and yourself could not attend the complex dependencies
tracks -- I think it would have been useful. I don't expect us to address a
resolution to probe ordering immediately -- but I am in the hopes we at least
can keep an open mind about the similarity of the problems and see if we can
aim for a clean elegant solution that might help both.

[2] https://lwn.net/Articles/625454/
[3] http://thread.gmane.org/gmane.linux.kernel/2087152

  Luis

^ permalink raw reply

* Re: [RFC] ARCv2: MCIP: Deprecate setting of affinity in Device Tree
From: Vineet Gupta @ 2016-11-08 20:57 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Yuriy Kolerov, linux-snps-arc@lists.infradead.org,
	Alexey.Brodkin@synopsys.com, tglx@linutronix.de,
	linux-kernel@vger.kernel.org
In-Reply-To: <1478261791-2793-1-git-send-email-yuriy.kolerov@synopsys.com>

+CC MarcZ

Hi Marc,

I have a question below

On 11/04/2016 05:17 AM, Yuriy Kolerov wrote:
> Ignore value of interrupt distribution mode for common interrupts in
> IDU since setting an affinity using value from Device Tree is deprecated
> in ARC. Originially it is done in idu_irq_xlate function and it is
> semantically wrong and does not guaranty that an affinity value will be
> set properly.
>
> However it is necessary to set a default affinity value manually for all
> common interrupts since initially all of them are disabled by IDU (a
> CPU mask for common interrupts is set to 0 after CPU reset) and in
> some cases the kernel cannot do it itself after initialization of
> endpoint devices (e.g. when IRQ chip below of IDU does not support
> setting of affinity and it cannot propagate an affinity value to IDU).
>
> P.S.
>
> Despite the fact that the patch works fine I would like to discuss
> few topics which are one way or another related to the original problem.
>
> Despite the fact that this patch works fine it does not work well when
> the intc below IDU does not support setting and propagating of affinity
> (e.g. GPIO intc in AXS103 configuration which does not implement a function
> for setting an affinity). That is why it is necessary to set an affinity
> manually for IDU - you cannot be sure that an affinity will be propagated
> to IDU properly even with support of hierarchical IRQ domains. But an
> affinity *must* be set because IDU common interrupts are disabled by
> default (registers which stores cpu masks are zeroes). In this patch
> I use this soulution in idu_irq_map() function:
>
>     irq_set_affinity(virq, cpu_online_mask);
>
> And there is a second problem. You cannot be sure that all cpus are
> online at the moment of calling idu_irq_map() that is why cpu_online_mask
> mask is used for setting of the initial affinity. It means that affinity
> of all common interrupts is unpredictable after kernel boot (of course it
> would be cool to distribute all common interrupts to all available cores
> by default).
>
> And the third topic is setting of affinity for AXS103 devices. Devices
> in AXS103 are connected to CPU through a chain of GPIO controllers which
> funnel all signals to 2 IDU common interrupts. Thus you cannot simply set
> an affinity for devices using something like:
>
>     echo 2 > /proc/irq/3/...
>
> because 1) GPIO contorrels in AXS103 does not support setting an affinity
> and 2) there is a logical explanation for it: if you want to set an affinity
> for a device connected to GPIO you produce a side affect by touchin an
> affinity for nearby devices.
>
> So if in case of AXS103 you must set an affinity not for devices but for
> GPIO controllers which are connected to IDU. Also you must to know a virq
> of the common interrupt line in IDU to set an affinity for it. Problem is
> that you cannot retrieve this value from /proc/interrupts for AXS103 even
> if support of hierarchical domains is implemented: /proc/interrupts does
> not show chained virqs and virqs without actions (e.g. all virqs of IDU).
>
> P.S.S.
>
> The question is how to solve all those problems properly.
>
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov@synopsys.com>
> ---
>  .../interrupt-controller/snps,archs-idu-intc.txt   |  2 ++
>  arch/arc/kernel/mcip.c                             | 35 +++++-----------------
>  2 files changed, 9 insertions(+), 28 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> index 0dcb7c7..e967e7f 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> @@ -15,6 +15,8 @@ Properties:
>    Second cell specifies the irq distribution mode to cores
>       0=Round Robin; 1=cpu0, 2=cpu1, 4=cpu2, 8=cpu3
>  
> +  The second cell in interrupts property is deprecated and ignored.
> +
>    intc accessed via the special ARC AUX register interface, hence "reg" property
>    is not specified.
>  
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index 90f9934..2f4b0dc 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -242,6 +242,7 @@ static int idu_irq_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t
>  {
>  	irq_set_chip_and_handler(virq, &idu_irq_chip, handle_level_irq);
>  	irq_set_status_flags(virq, IRQ_MOVE_PCNTXT);
> +	irq_set_affinity(virq, cpu_online_mask);

So as discussed in a prior thread, we no longer support setting the default
affinity via DT (so removed from xlate function). However we are now using the
domain map function to set "seed" affinity based on cpu_online_mask(). Will that
be the wrong place again to do this too.
Also as Yuriy notes above, it is not guaranteed that all cpus will be online when
the map function is called. So then better to set to boot cpu only : wondering
what approach you guys take for ARM ?

Thx,
-Vineet

>  
>  	return 0;
>  }
> @@ -250,36 +251,14 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *n,
>  			 const u32 *intspec, unsigned int intsize,
>  			 irq_hw_number_t *out_hwirq, unsigned int *out_type)
>  {
> -	irq_hw_number_t hwirq = *out_hwirq = intspec[0];
> -	int distri = intspec[1];
> -	unsigned long flags;
> -
> +	/*
> +	 * Ignore value of interrupt distribution mode for common interrupts in
> +	 * IDU which resides in intspec[1] since setting an affinity using value
> +	 * from Device Tree is deprecated in ARC.
> +	 */
> +	*out_hwirq = intspec[0];
>  	*out_type = IRQ_TYPE_NONE;
>  
> -	/* XXX: validate distribution scheme again online cpu mask */
> -	if (distri == 0) {
> -		/* 0 - Round Robin to all cpus, otherwise 1 bit per core */
> -		raw_spin_lock_irqsave(&mcip_lock, flags);
> -		idu_set_dest(hwirq, BIT(num_online_cpus()) - 1);
> -		idu_set_mode(hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_RR);
> -		raw_spin_unlock_irqrestore(&mcip_lock, flags);
> -	} else {
> -		/*
> -		 * DEST based distribution for Level Triggered intr can only
> -		 * have 1 CPU, so generalize it to always contain 1 cpu
> -		 */
> -		int cpu = ffs(distri);
> -
> -		if (cpu != fls(distri))
> -			pr_warn("IDU irq %lx distri mode set to cpu %x\n",
> -				hwirq, cpu);
> -
> -		raw_spin_lock_irqsave(&mcip_lock, flags);
> -		idu_set_dest(hwirq, cpu);
> -		idu_set_mode(hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_DEST);
> -		raw_spin_unlock_irqrestore(&mcip_lock, flags);
> -	}
> -
>  	return 0;
>  }
>  

^ permalink raw reply

* [RFC] ARCv2: MCIP: Deprecate setting of affinity in Device Tree
From: Vineet Gupta @ 2016-11-08 20:57 UTC (permalink / raw)
  To: linux-snps-arc
In-Reply-To: <1478261791-2793-1-git-send-email-yuriy.kolerov@synopsys.com>

+CC MarcZ

Hi Marc,

I have a question below

On 11/04/2016 05:17 AM, Yuriy Kolerov wrote:
> Ignore value of interrupt distribution mode for common interrupts in
> IDU since setting an affinity using value from Device Tree is deprecated
> in ARC. Originially it is done in idu_irq_xlate function and it is
> semantically wrong and does not guaranty that an affinity value will be
> set properly.
>
> However it is necessary to set a default affinity value manually for all
> common interrupts since initially all of them are disabled by IDU (a
> CPU mask for common interrupts is set to 0 after CPU reset) and in
> some cases the kernel cannot do it itself after initialization of
> endpoint devices (e.g. when IRQ chip below of IDU does not support
> setting of affinity and it cannot propagate an affinity value to IDU).
>
> P.S.
>
> Despite the fact that the patch works fine I would like to discuss
> few topics which are one way or another related to the original problem.
>
> Despite the fact that this patch works fine it does not work well when
> the intc below IDU does not support setting and propagating of affinity
> (e.g. GPIO intc in AXS103 configuration which does not implement a function
> for setting an affinity). That is why it is necessary to set an affinity
> manually for IDU - you cannot be sure that an affinity will be propagated
> to IDU properly even with support of hierarchical IRQ domains. But an
> affinity *must* be set because IDU common interrupts are disabled by
> default (registers which stores cpu masks are zeroes). In this patch
> I use this soulution in idu_irq_map() function:
>
>     irq_set_affinity(virq, cpu_online_mask);
>
> And there is a second problem. You cannot be sure that all cpus are
> online at the moment of calling idu_irq_map() that is why cpu_online_mask
> mask is used for setting of the initial affinity. It means that affinity
> of all common interrupts is unpredictable after kernel boot (of course it
> would be cool to distribute all common interrupts to all available cores
> by default).
>
> And the third topic is setting of affinity for AXS103 devices. Devices
> in AXS103 are connected to CPU through a chain of GPIO controllers which
> funnel all signals to 2 IDU common interrupts. Thus you cannot simply set
> an affinity for devices using something like:
>
>     echo 2 > /proc/irq/3/...
>
> because 1) GPIO contorrels in AXS103 does not support setting an affinity
> and 2) there is a logical explanation for it: if you want to set an affinity
> for a device connected to GPIO you produce a side affect by touchin an
> affinity for nearby devices.
>
> So if in case of AXS103 you must set an affinity not for devices but for
> GPIO controllers which are connected to IDU. Also you must to know a virq
> of the common interrupt line in IDU to set an affinity for it. Problem is
> that you cannot retrieve this value from /proc/interrupts for AXS103 even
> if support of hierarchical domains is implemented: /proc/interrupts does
> not show chained virqs and virqs without actions (e.g. all virqs of IDU).
>
> P.S.S.
>
> The question is how to solve all those problems properly.
>
> Signed-off-by: Yuriy Kolerov <yuriy.kolerov at synopsys.com>
> ---
>  .../interrupt-controller/snps,archs-idu-intc.txt   |  2 ++
>  arch/arc/kernel/mcip.c                             | 35 +++++-----------------
>  2 files changed, 9 insertions(+), 28 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> index 0dcb7c7..e967e7f 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/snps,archs-idu-intc.txt
> @@ -15,6 +15,8 @@ Properties:
>    Second cell specifies the irq distribution mode to cores
>       0=Round Robin; 1=cpu0, 2=cpu1, 4=cpu2, 8=cpu3
>  
> +  The second cell in interrupts property is deprecated and ignored.
> +
>    intc accessed via the special ARC AUX register interface, hence "reg" property
>    is not specified.
>  
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index 90f9934..2f4b0dc 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -242,6 +242,7 @@ static int idu_irq_map(struct irq_domain *d, unsigned int virq, irq_hw_number_t
>  {
>  	irq_set_chip_and_handler(virq, &idu_irq_chip, handle_level_irq);
>  	irq_set_status_flags(virq, IRQ_MOVE_PCNTXT);
> +	irq_set_affinity(virq, cpu_online_mask);

So as discussed in a prior thread, we no longer support setting the default
affinity via DT (so removed from xlate function). However we are now using the
domain map function to set "seed" affinity based on cpu_online_mask(). Will that
be the wrong place again to do this too.
Also as Yuriy notes above, it is not guaranteed that all cpus will be online when
the map function is called. So then better to set to boot cpu only : wondering
what approach you guys take for ARM ?

Thx,
-Vineet

>  
>  	return 0;
>  }
> @@ -250,36 +251,14 @@ static int idu_irq_xlate(struct irq_domain *d, struct device_node *n,
>  			 const u32 *intspec, unsigned int intsize,
>  			 irq_hw_number_t *out_hwirq, unsigned int *out_type)
>  {
> -	irq_hw_number_t hwirq = *out_hwirq = intspec[0];
> -	int distri = intspec[1];
> -	unsigned long flags;
> -
> +	/*
> +	 * Ignore value of interrupt distribution mode for common interrupts in
> +	 * IDU which resides in intspec[1] since setting an affinity using value
> +	 * from Device Tree is deprecated in ARC.
> +	 */
> +	*out_hwirq = intspec[0];
>  	*out_type = IRQ_TYPE_NONE;
>  
> -	/* XXX: validate distribution scheme again online cpu mask */
> -	if (distri == 0) {
> -		/* 0 - Round Robin to all cpus, otherwise 1 bit per core */
> -		raw_spin_lock_irqsave(&mcip_lock, flags);
> -		idu_set_dest(hwirq, BIT(num_online_cpus()) - 1);
> -		idu_set_mode(hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_RR);
> -		raw_spin_unlock_irqrestore(&mcip_lock, flags);
> -	} else {
> -		/*
> -		 * DEST based distribution for Level Triggered intr can only
> -		 * have 1 CPU, so generalize it to always contain 1 cpu
> -		 */
> -		int cpu = ffs(distri);
> -
> -		if (cpu != fls(distri))
> -			pr_warn("IDU irq %lx distri mode set to cpu %x\n",
> -				hwirq, cpu);
> -
> -		raw_spin_lock_irqsave(&mcip_lock, flags);
> -		idu_set_dest(hwirq, cpu);
> -		idu_set_mode(hwirq, IDU_M_TRIG_LEVEL, IDU_M_DISTRI_DEST);
> -		raw_spin_unlock_irqrestore(&mcip_lock, flags);
> -	}
> -
>  	return 0;
>  }
>  

^ permalink raw reply

* [PATCH 2/2] ASoC: axentia: tse850: add ASoC driver for the Axentia TSE-850
From: Peter Rosin @ 2016-11-08 16:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: Peter Rosin, Liam Girdwood, Mark Brown, Rob Herring, Mark Rutland,
	Jaroslav Kysela, Takashi Iwai, alsa-devel, devicetree
In-Reply-To: <1478622057-12426-1-git-send-email-peda@axentia.se>

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

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f2ebf3ab51a..e966dfa79680 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2323,6 +2323,7 @@ M:	Peter Rosin <peda@axentia.se>
 L:	alsa-devel@alsa-project.org (moderated for non-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/sound/axentia,*
+F:	sound/soc/axentia/
 
 AZ6007 DVB DRIVER
 M:	Mauro Carvalho Chehab <mchehab@s-opensource.com>
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 182d92efc7c8..5e2f9e58710b 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -41,6 +41,7 @@ source "sound/soc/adi/Kconfig"
 source "sound/soc/amd/Kconfig"
 source "sound/soc/atmel/Kconfig"
 source "sound/soc/au1x/Kconfig"
+source "sound/soc/axentia/Kconfig"
 source "sound/soc/bcm/Kconfig"
 source "sound/soc/blackfin/Kconfig"
 source "sound/soc/cirrus/Kconfig"
diff --git a/sound/soc/Makefile b/sound/soc/Makefile
index 9a30f21d16ee..dcdd41358e2a 100644
--- a/sound/soc/Makefile
+++ b/sound/soc/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_SND_SOC)	+= adi/
 obj-$(CONFIG_SND_SOC)	+= amd/
 obj-$(CONFIG_SND_SOC)	+= atmel/
 obj-$(CONFIG_SND_SOC)	+= au1x/
+obj-$(CONFIG_SND_SOC)	+= axentia/
 obj-$(CONFIG_SND_SOC)	+= bcm/
 obj-$(CONFIG_SND_SOC)	+= blackfin/
 obj-$(CONFIG_SND_SOC)	+= cirrus/
diff --git a/sound/soc/axentia/Kconfig b/sound/soc/axentia/Kconfig
new file mode 100644
index 000000000000..7c760edd4bed
--- /dev/null
+++ b/sound/soc/axentia/Kconfig
@@ -0,0 +1,10 @@
+config SND_SOC_AXENTIA_TSE850_PCM5142
+	tristate "ASoC driver for the Axentia TSE-850"
+	depends on ARCH_AT91 && OF
+	select ATMEL_SSC
+	select SND_ATMEL_SOC
+	select SND_ATMEL_SOC_SSC_DMA
+	select SND_SOC_PCM512x_I2C
+	help
+	  Say Y if you want to add support for the ASoC driver for the
+	  Axentia TSE-850 with a PCM5142 codec.
diff --git a/sound/soc/axentia/Makefile b/sound/soc/axentia/Makefile
new file mode 100644
index 000000000000..d4ef52915809
--- /dev/null
+++ b/sound/soc/axentia/Makefile
@@ -0,0 +1,3 @@
+snd-soc-tse850-pcm5142-objs := tse850-pcm5142.o
+
+obj-$(CONFIG_SND_SOC_AXENTIA_TSE850_PCM5142) += snd-soc-tse850-pcm5142.o
diff --git a/sound/soc/axentia/tse850-pcm5142.c b/sound/soc/axentia/tse850-pcm5142.c
new file mode 100644
index 000000000000..1865f1eea554
--- /dev/null
+++ b/sound/soc/axentia/tse850-pcm5142.c
@@ -0,0 +1,504 @@
+/*
+ * TSE-850 audio - ASoC driver for the Axentia TSE-850 with a PCM5142 codec
+ *
+ * Copyright (C) 2016 Axentia Technologies AB
+ *
+ * Author: Peter Rosin <peda@axentia.se>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/*
+ *               loop1 relays
+ *   IN1 +---o  +------------+  o---+ OUT1
+ *            \                /
+ *             +              +
+ *             |   /          |
+ *             +--o  +--.     |
+ *             |  add   |     |
+ *             |        V     |
+ *             |      .---.   |
+ *   DAC +----------->|Sum|---+
+ *             |      '---'   |
+ *             |              |
+ *             +              +
+ *
+ *   IN2 +---o--+------------+--o---+ OUT2
+ *               loop2 relays
+ *
+ * The 'loop1' gpio pin controlls two relays, which are either in loop
+ * position, meaning that input and output are directly connected, or
+ * they are in mixer position, meaning that the signal is passed through
+ * the 'Sum' mixer. Similarly for 'loop2'.
+ *
+ * In the above, the 'loop1' relays are inactive, thus feeding IN1 to the
+ * mixer (if 'add' is active) and feeding the mixer output to OUT1. The
+ * 'loop2' relays are active, short-cutting the TSE-850 from channel 2.
+ * IN1, IN2, OUT1 and OUT2 are TSE-850 connectors and DAC is the PCB name
+ * of the (filtered) output from the PCM5142 codec.
+ */
+
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/regulator/consumer.h>
+
+#include <sound/soc.h>
+#include <sound/pcm_params.h>
+
+#include "../atmel/atmel_ssc_dai.h"
+
+struct tse850_priv {
+	int ssc_id;
+
+	struct gpio_desc *add;
+	struct gpio_desc *loop1;
+	struct gpio_desc *loop2;
+
+	struct regulator *ana;
+};
+
+static int tse850_get_mux1(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	ret = gpiod_get_value(tse850->loop1);
+	if (ret < 0)
+		return ret;
+
+	ucontrol->value.enumerated.item[0] = ret;
+
+	return 0;
+}
+
+static int tse850_put_mux1(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int val = ucontrol->value.enumerated.item[0];
+
+	if (val >= e->items)
+		return -EINVAL;
+
+	gpiod_set_value(tse850->loop1, val);
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+static int tse850_get_mux2(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	ret = gpiod_get_value(tse850->loop2);
+	if (ret < 0)
+		return ret;
+
+	ucontrol->value.enumerated.item[0] = ret;
+
+	return 0;
+}
+
+static int tse850_put_mux2(struct snd_kcontrol *kctrl,
+			   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int val = ucontrol->value.enumerated.item[0];
+
+	if (val >= e->items)
+		return -EINVAL;
+
+	gpiod_set_value(tse850->loop2, val);
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+int tse850_get_mix(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	ret = gpiod_get_value(tse850->add);
+	if (ret < 0)
+		return ret;
+
+	ucontrol->value.enumerated.item[0] = ret;
+
+	return 0;
+}
+
+int tse850_put_mix(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int old;
+	int connect;
+
+	connect = !!ucontrol->value.integer.value[0];
+	old = gpiod_get_value(tse850->add);
+
+	if (old == connect)
+		return 0;
+
+	/*
+	 * Hmmm, this gpiod_set_value call should probably happen
+	 * inside snd_soc_dapm_mixer_update_power in the loop.
+	 */
+	gpiod_set_value(tse850->add, connect);
+
+	snd_soc_dapm_mixer_update_power(dapm, kctrl, connect, NULL);
+	return 1;
+}
+
+int tse850_get_ana(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	ret = regulator_get_voltage(tse850->ana);
+	if (ret < 0)
+		return ret;
+
+	if (ret < 11000000)
+		ret = 11000000;
+	else if (ret > 20000000)
+		ret = 20000000;
+	ret -= 11000000;
+	ret = (ret + 500000) / 1000000;
+
+	ucontrol->value.enumerated.item[0] = ret;
+
+	return 0;
+}
+
+int tse850_put_ana(struct snd_kcontrol *kctrl,
+		   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_dapm(kctrl);
+	struct snd_soc_card *card = dapm->card;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+	struct soc_enum *e = (struct soc_enum *)kctrl->private_value;
+	unsigned int uV = ucontrol->value.enumerated.item[0];
+	int ret;
+
+	if (uV >= e->items)
+		return -EINVAL;
+
+	/*
+	 * Map enum zero (Low) to 2 volts on the regulator, do this since
+	 * the ana regulator is supplied by the system 12V voltage and
+	 * requesting anything below the system voltage causes the system
+	 * voltage to be passed through the regulator. Also, the ana
+	 * regulator induces noise when requesting voltages near the
+	 * system voltage. So, by mapping Low to 2V, that noise is
+	 * eliminated when all that is needed is 12V (the system voltage).
+	 */
+	if (uV)
+		uV = 11000000 + (1000000 * uV);
+	else
+		uV = 2000000;
+
+	ret = regulator_set_voltage(tse850->ana, uV, uV);
+	if (ret < 0)
+		return ret;
+
+	return snd_soc_dapm_put_enum_double(kctrl, ucontrol);
+}
+
+static const char * const mux_text[] = { "Mixer", "Loop" };
+
+static const struct soc_enum mux_enum =
+	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, 2, mux_text);
+
+static const struct snd_kcontrol_new mux1 =
+	SOC_DAPM_ENUM_EXT("MUX1", mux_enum, tse850_get_mux1, tse850_put_mux1);
+
+static const struct snd_kcontrol_new mux2 =
+	SOC_DAPM_ENUM_EXT("MUX2", mux_enum, tse850_get_mux2, tse850_put_mux2);
+
+#define TSE850_DAPM_SINGLE_EXT(xname, reg, shift, max, invert, xget, xput) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
+	.info = snd_soc_info_volsw, \
+	.get = xget, \
+	.put = xput, \
+	.private_value = SOC_SINGLE_VALUE(reg, shift, max, invert, 0) }
+
+static const struct snd_kcontrol_new mix[] = {
+	TSE850_DAPM_SINGLE_EXT("IN Switch", SND_SOC_NOPM, 0, 1, 0,
+			       tse850_get_mix, tse850_put_mix),
+};
+
+static const char * const ana_text[] = {
+	"Low", "12V", "13V", "14V", "15V", "16V", "17V", "18V", "19V", "20V"
+};
+
+static const struct soc_enum ana_enum =
+	SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, 9, ana_text);
+
+static const struct snd_kcontrol_new out =
+	SOC_DAPM_ENUM_EXT("ANA", ana_enum, tse850_get_ana, tse850_put_ana);
+
+static const struct snd_soc_dapm_widget tse850_dapm_widgets[] = {
+	SND_SOC_DAPM_LINE("OUT1", NULL),
+	SND_SOC_DAPM_LINE("OUT2", NULL),
+	SND_SOC_DAPM_LINE("IN1", NULL),
+	SND_SOC_DAPM_LINE("IN2", NULL),
+	SND_SOC_DAPM_INPUT("DAC"),
+	SND_SOC_DAPM_AIF_IN("AIFINL", "Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("AIFINR", "Playback", 1, SND_SOC_NOPM, 0, 0),
+	SOC_MIXER_ARRAY("MIX", SND_SOC_NOPM, 0, 0, mix),
+	SND_SOC_DAPM_MUX("MUX1", SND_SOC_NOPM, 0, 0, &mux1),
+	SND_SOC_DAPM_MUX("MUX2", SND_SOC_NOPM, 0, 0, &mux2),
+	SND_SOC_DAPM_OUT_DRV("OUT", SND_SOC_NOPM, 0, 0, &out, 1),
+};
+
+/*
+ * These connections are not entirely correct, since both IN1 and IN2
+ * are always fed to MIX (if the "IN switch" is set so), i.e. without
+ * regard to the loop1 and loop2 relays that according to this only
+ * control MUX1 and MUX2 but in fact also control how the input signals
+ * are routed.
+ * But, 1) I don't know how to do it right, and 2) it doesn't seem to
+ * matter in practice since nothing is powered in those sections anyway.
+ */
+static const struct snd_soc_dapm_route tse850_intercon[] = {
+	{ "OUT1", NULL, "MUX1" },
+	{ "OUT2", NULL, "MUX2" },
+
+	{ "MUX1", "Loop",  "IN1" },
+	{ "MUX1", "Mixer", "OUT" },
+
+	{ "MUX2", "Loop",  "IN2" },
+	{ "MUX2", "Mixer", "OUT" },
+
+	{ "OUT", NULL, "MIX" },
+
+	{ "MIX", NULL, "DAC" },
+	{ "MIX", "IN Switch", "IN1" },
+	{ "MIX", "IN Switch", "IN2" },
+
+	/* connect board input to the codec left channel output pin */
+	{ "DAC", NULL, "OUTL" },
+};
+
+static int tse850_hw_params(struct snd_pcm_substream *substream,
+			    struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct device *dev = rtd->dev;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	int dir = substream->stream != SNDRV_PCM_STREAM_PLAYBACK;
+	int div_id = dir ? ATMEL_SSC_RCMR_PERIOD : ATMEL_SSC_TCMR_PERIOD;
+	int period = snd_soc_params_to_frame_size(params) / 2 - 1;
+	int ret;
+
+	ret = snd_soc_dai_set_clkdiv(cpu_dai, div_id, period);
+	if (ret < 0) {
+		dev_err(dev, "failed to set cpu dai lrclk %d divider\n", dir);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_ops tse850_ops = {
+	.hw_params = tse850_hw_params,
+};
+
+static int tse850_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_dapm_context *dapm = &rtd->card->dapm;
+
+	return snd_soc_dapm_add_routes(dapm, tse850_intercon,
+				       ARRAY_SIZE(tse850_intercon));
+}
+
+static struct snd_soc_dai_link tse850_dailink = {
+	.name = "TSE-850",
+	.stream_name = "TSE-850-PCM",
+	.codec_dai_name = "pcm512x-hifi",
+	.dai_fmt = SND_SOC_DAIFMT_I2S
+		 | SND_SOC_DAIFMT_NB_NF
+		 | SND_SOC_DAIFMT_CBM_CFS,
+	.init = tse850_init,
+	.ops = &tse850_ops,
+};
+
+static struct snd_soc_card tse850_card = {
+	.name = "TSE-850-ASoC",
+	.owner = THIS_MODULE,
+	.dai_link = &tse850_dailink,
+	.num_links = 1,
+	.dapm_widgets = tse850_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(tse850_dapm_widgets),
+	.fully_routed = true,
+};
+
+static int tse850_dt_init(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *codec_np, *cpu_np;
+	struct snd_soc_card *card = &tse850_card;
+	struct snd_soc_dai_link *dailink = &tse850_dailink;
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	if (!np) {
+		dev_err(&pdev->dev, "only device tree supported\n");
+		return -EINVAL;
+	}
+
+	cpu_np = of_parse_phandle(np, "axentia,ssc-controller", 0);
+	if (!cpu_np) {
+		dev_err(&pdev->dev, "failed to get dai and pcm info\n");
+		return -EINVAL;
+	}
+	dailink->cpu_of_node = cpu_np;
+	dailink->platform_of_node = cpu_np;
+	tse850->ssc_id = of_alias_get_id(cpu_np, "ssc");
+	of_node_put(cpu_np);
+
+	codec_np = of_parse_phandle(np, "axentia,audio-codec", 0);
+	if (!codec_np) {
+		dev_err(&pdev->dev, "failed to get codec info\n");
+		return -EINVAL;
+	}
+	dailink->codec_of_node = codec_np;
+	of_node_put(codec_np);
+
+	return 0;
+}
+
+static int tse850_probe(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = &tse850_card;
+	struct device *dev = card->dev = &pdev->dev;
+	struct tse850_priv *tse850;
+	int ret;
+
+	tse850 = devm_kzalloc(dev, sizeof(*tse850), GFP_KERNEL);
+	if (!tse850)
+		return -ENOMEM;
+
+	snd_soc_card_set_drvdata(card, tse850);
+
+	ret = tse850_dt_init(pdev);
+	if (ret) {
+		dev_err(dev, "failed to init dt info\n");
+		return ret;
+	}
+
+	tse850->add = devm_gpiod_get(dev, "axentia,add", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->add)) {
+		if (PTR_ERR(tse850->add) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'add' gpio\n");
+		return PTR_ERR(tse850->add);
+	}
+
+	tse850->loop1 = devm_gpiod_get(dev, "axentia,loop1", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->loop1)) {
+		if (PTR_ERR(tse850->loop1) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'loop1' gpio\n");
+		return PTR_ERR(tse850->loop1);
+	}
+
+	tse850->loop2 = devm_gpiod_get(dev, "axentia,loop2", GPIOD_OUT_HIGH);
+	if (IS_ERR(tse850->loop2)) {
+		if (PTR_ERR(tse850->loop2) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'loop2' gpio\n");
+		return PTR_ERR(tse850->loop2);
+	}
+
+	tse850->ana = devm_regulator_get(dev, "axentia,ana");
+	if (IS_ERR(tse850->ana)) {
+		if (PTR_ERR(tse850->ana) != -EPROBE_DEFER)
+			dev_err(dev, "failed to get 'ana' regulator\n");
+		return PTR_ERR(tse850->ana);
+	}
+
+	ret = regulator_enable(tse850->ana);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable the 'ana' regulator\n");
+		return ret;
+	}
+
+	ret = atmel_ssc_set_audio(tse850->ssc_id);
+	if (ret != 0) {
+		dev_err(dev,
+			"failed to set SSC %d for audio\n", tse850->ssc_id);
+		goto err_disable_ana;
+	}
+
+	ret = snd_soc_register_card(card);
+	if (ret) {
+		dev_err(dev, "snd_soc_register_card failed\n");
+		goto err_put_audio;
+	}
+
+	return 0;
+
+err_put_audio:
+	atmel_ssc_put_audio(tse850->ssc_id);
+err_disable_ana:
+	regulator_disable(tse850->ana);
+	return ret;
+}
+
+static int tse850_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+	struct tse850_priv *tse850 = snd_soc_card_get_drvdata(card);
+
+	snd_soc_unregister_card(card);
+	atmel_ssc_put_audio(tse850->ssc_id);
+	regulator_disable(tse850->ana);
+
+	return 0;
+}
+
+static const struct of_device_id tse850_dt_ids[] = {
+	{ .compatible = "axentia,tse850-pcm5142", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tse850_dt_ids);
+
+static struct platform_driver tse850_driver = {
+	.driver = {
+		.name = "axentia-tse850-pcm5142",
+		.of_match_table = of_match_ptr(tse850_dt_ids),
+	},
+	.probe = tse850_probe,
+	.remove = tse850_remove,
+};
+
+module_platform_driver(tse850_driver);
+
+/* Module information */
+MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
+MODULE_DESCRIPTION("ALSA SoC driver for TSE-850 with PCM5142 codec");
+MODULE_LICENSE("GPL");
-- 
2.1.4

^ permalink raw reply related

* Re: GPF in sidtab_context_to_sid
From: Paul Moore @ 2016-11-08 20:56 UTC (permalink / raw)
  To: Roberts, William C; +Cc: selinux@tycho.nsa.gov
In-Reply-To: <476DC76E7D1DF2438D32BFADF679FC561CD2178D@ORSMSX103.amr.corp.intel.com>

On Tue, Nov 8, 2016 at 1:26 PM, Roberts, William C
<william.c.roberts@intel.com> wrote:
> I found a very similar oops online:
>
> http://oops.kernel.org/oops/general-protection-fault-in-sidtab_context_to_sid/
>
> Has anyone encountered this bug?
>
> I had something reported to me very similar where the faulting instruction
> was:
>
> 0xffffffff8133c81e <+174>:   mov    0x14(%r12),%eax
>
> Addr2line on vmlinux produced:
>
> $ addr2line -f -e ./vmlinux ffffffff8133c81e
> context_cmp
> kernel/cht/security/selinux/ss/context.h:152

I'm guessing you don't have a reproducer?

It looks like both these kernels are older (3.x), have you seen this
on anything recent?

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* [PATCH] testsuite: fix test_array_sort pointers inderection
From: Yauheni Kaliuta @ 2016-11-08 20:57 UTC (permalink / raw)
  To: linux-modules; +Cc: Lucas De Marchi, Jessica Yu

The array elements in the tests are strings, what means "char *"
in С. The comparation funtion takes pointers to the elements, so
the arguments become "char **". It means, that strcmp() cannot be
used directrly.

The patch creates a wrapper on strcmp() which perfoms
dereferencing of the "char **" to supply the actual strings to
strcmp(), and uses the wrapper as a comparation function for the
qsort() call.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 testsuite/test-array.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/testsuite/test-array.c b/testsuite/test-array.c
index 3c72a8a109c5..ef1e1e9922c9 100644
--- a/testsuite/test-array.c
+++ b/testsuite/test-array.c
@@ -90,6 +90,13 @@ static int test_array_append_unique(const struct test *t)
 DEFINE_TEST(test_array_append_unique,
 		.description = "test array append unique");
 
+static int strptrcmp(const void *pa, const void *pb) {
+	const char *a = *(const char **)pa;
+	const char *b = *(const char **)pb;
+
+	return strcmp(a, b);
+}
+
 static int test_array_sort(const struct test *t)
 {
 	struct array array;
@@ -104,7 +111,7 @@ static int test_array_sort(const struct test *t)
 	array_append(&array, c2);
 	array_append(&array, c3);
 	array_append(&array, c1);
-	array_sort(&array, (int (*)(const void *a, const void *b)) strcmp);
+	array_sort(&array, strptrcmp);
 	assert_return(array.count == 6, EXIT_FAILURE);
 	assert_return(array.array[0] == c1, EXIT_FAILURE);
 	assert_return(array.array[1] == c1, EXIT_FAILURE);
-- 
2.7.4


^ permalink raw reply related

* Re: How to handle meta-intel/openembedded repos with multiple developers
From: Bas Mevissen @ 2016-11-08 20:54 UTC (permalink / raw)
  To: yocto
In-Reply-To: <CAEjjReLCNOKCq5dawyvbFyH=M5B1eot36SLuwCXMFJ1r_o-fUg@mail.gmail.com>

On 07/11/2016 20:31, Chris Z. wrote:
> Hi,
> 
> How you store your project configuration ? How you prepare workspace
> (download each layer separately)?
> Basic stuff, SW release should be reproducible (in easy way). Store
> somewhere used hash of each piece or use tags. Non company assets should
> be already somehow tagged or you use HEAD or master/-next ?
> 

I think the best option is to use the Google Repo tool. You write an xml
file that specifies the layers to download and which commit, branch or
tag to checkout. The Freescale official and community project do it like
this, just like Android.

-- 
Bas.


^ permalink raw reply

* [Buildroot] [PATCH 2/6] package/pseudo: enforce the host bitness
From: Arnout Vandecappelle @ 2016-11-08 20:54 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <20161108172912.GC3554@free.fr>



On 08-11-16 18:29, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2016-11-07 23:16 +0100, Arnout Vandecappelle spake thusly:
>>
>>
>> On 07-11-16 22:23, Yann E. MORIN wrote:
>>> pseudo can detect the host bitness, but is not sure about it: it checks
>>> what type of file /bin/sh is, using file(1).
>>>
>>> However, in some conditions, /bin/sh can be of a different bitness than
>>> the rest of the system (weird, but not impossible), which causes build
>>> issues.
>>>
>>> Just enforce the bitness, so that pseudo needs not (wrongly) guess it.
>>>
>>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> Cc: Ga?l PORTAY <gael.portay@savoirfairelinux.com>
>>> Cc: Patrick Keroulas <patrick.keroulas@savoirfairelinux.com>
>>> Cc: Erico Nunes <nunes.erico@gmail.com>
>>> Cc: Julien BOIBESSOT <julien.boibessot@free.fr>
>>> ---
>>>  package/pseudo/pseudo.mk | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/package/pseudo/pseudo.mk b/package/pseudo/pseudo.mk
>>> index 67538a6..3b0ba86 100644
>>> --- a/package/pseudo/pseudo.mk
>>> +++ b/package/pseudo/pseudo.mk
>>> @@ -17,6 +17,7 @@ HOST_PSEUDO_DEPENDENCIES = host-attr host-sqlite
>>>  # configure script is not generated by autoconf, so passing --libdir
>>>  # is necessary, even if the infrastructure passes --prefix already.
>>>  HOST_PSEUDO_CONF_OPTS = \
>>> +	--bits=$(HOSTARCH_BITS) \
>>
>>  I would be just as happy with
>> 	--bits=$(if $(filter %64,$(HOSTARCH)),32,64)
> 
> Yep, that's nice. Except it's reversed, isn't it? I.e. if we find '64'
> in HOSTARCH, then we want to use '64', not '32', right? So:
> 
>     --bits=$(if $(filter %64,$(HOSTARCH)),64,32)

 Ugh, yes, of course, I had filter-out in my head for some reason. Ah, no, it's
because the condition in patch 1/6 was reversed that I was confused.

 Regards,
 Arnout

> 
> (Unless I missed something...)
> 
> Regards,
> Yann E. MORIN.
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply

* Re: QCA988X firmware crash when sending in monitor mode
From: Ouri Lipner @ 2016-11-08 20:54 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k@lists.infradead.org
In-Reply-To: <CA+BoTQm0fE6DAd1wNXnZ_M1r_PbitQETwV+ku8Ue5h6kW67SCw@mail.gmail.com>

I’m aware of what the wiki says, but was under the impression this is outdated.
Recent firmware files have a raw mode capability, and there is also code in the driver to handle it.

Adding an AP alongside the monitor vdev is an interesting idea, thanks.
I’ll have to disable beacon send etc...

On 08/11/2016, 01:03, "Michal Kazior" <michal.kazior@tieto.com> wrote:

    On 7 November 2016 at 15:22, Ouri Lipner <lipner@equus-tech.com> wrote:
    > Hi all,
    > I’m consistently encountering issues with raw wifi packet injection.
    >
    > Sending a raw packet in monitor mode keeps crashing the firmware.
    > This happens consistently with every firmware that supports raw mode.
    > Currently on kernel 4.9-rc2 but older drivers behave the same.
    > Is this an issues with a missing parameter (e.g. in RadioTap) or a real problem with the firmware?
    
    To the best of my knowledge firmware does not support frame injection
    on monitor vdev. Therefore this is expected. Moreover this limitation
    is mentioned on ath10k wiki:
    https://wireless.wiki.kernel.org/en/users/drivers/ath10k
    
    
    > Any ideas?
    
    You could probably hack up the driver to create and use AP or STA vdev
    alongside monitor vdev to use it's vdev_id for frame injection. I'm
    not aware of anyone trying that out so far though.
    
    
    Michał
    

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply

* [U-Boot] Pull request, u-boot-tegra/master
From: Tom Rini @ 2016-11-08 20:53 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <CACih+8XphQAvUuiP6yEGTV=vZs9rnVHVe1bm2MLCJg0pmZgg4g@mail.gmail.com>

On Tue, Nov 08, 2016 at 08:34:04AM -0700, Tom Warren wrote:

> Tom,
> 
> Please pull u-boot-tegra/master into U-Boot/master. Thanks!
> 
> All Tegra builds are OK, and Stephen's automated test system reports that
> all tests pass.
> 
> The following changes since commit 4b6035da482cccda06aeb419634f99937c9fc783:
> 
>   mx6sabresd: Make Ethernet functional again (2016-11-06 06:59:27 -0500)
> 
> are available in the git repository at:
> 
>   git://git.denx.de/u-boot-tegra.git master
> 
> for you to fetch changes up to a8d0526133e542ea93a741fd18833e571e817775:
> 
>   ARM: tegra186: call secure monitor for all cache-wide ops (2016-11-07
> 14:36:29 -0800)
> 

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161108/f3962eed/attachment.sig>

^ permalink raw reply

* [PATCH] libyaml: Enable nativesdk bake
From: Brad Bishop @ 2016-11-08 20:45 UTC (permalink / raw)
  To: openembedded-core

Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
 meta/recipes-support/libyaml/libyaml_0.1.7.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-support/libyaml/libyaml_0.1.7.bb b/meta/recipes-support/libyaml/libyaml_0.1.7.bb
index 5c422ef..b167f03 100644
--- a/meta/recipes-support/libyaml/libyaml_0.1.7.bb
+++ b/meta/recipes-support/libyaml/libyaml_0.1.7.bb
@@ -17,4 +17,4 @@ S = "${WORKDIR}/yaml-${PV}"
 
 inherit autotools
 
-BBCLASSEXTEND = "native"
+BBCLASSEXTEND = "native nativesdk"
-- 
1.8.3.1


^ permalink raw reply related

* Re: Richacl and stored but ignored permissions
From: Andreas Gruenbacher @ 2016-11-08 20:53 UTC (permalink / raw)
  To: Steve French; +Cc: linux-fsdevel, Anne Marie Merritt, Weston Andros Adamson
In-Reply-To: <CAH2r5mvtWv6D4zmcFWL5Yowiy+wQ5mGgd1mRWNCT4bBGnrQLQA@mail.gmail.com>

Steve,

please never cross-post to the samba-technical mailing list. It's a
members-only, unmoderated list, and whenever anybody who isn't a
member replies to an email that has samba-technical cross-posted, they
get a nasty mail delivery error:

550-Please subscribe the address agruenba@redhat.com to the samba-technical
550-mailing list before posting. See
<https://lists.samba.org/mailman/listinfo/samba-technical>.
550-You can also write a mail to samba-technical-owner@lists.samba.org and ask
550 to get on the list of accepted non-members to be able to send mail here.

Thanks,
Andreas

^ permalink raw reply

* Re: [PATCHv2] ARM: dts: socfpga: add specific compatible strings for boards
From: Dinh Nguyen @ 2016-11-08 20:53 UTC (permalink / raw)
  To: Olof Johansson
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	dinguyen-yzvPICuk2ABMcg4IHK0kFoH6Mc4MB0Vx,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	vladimir_zapolskiy-nmGgyN9QBj3QT0dZR+AlfA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161101222352.GA6328@localhost>



On 11/01/2016 05:23 PM, Olof Johansson wrote:
> On Tue, Nov 01, 2016 at 03:56:52PM -0500, Dinh Nguyen wrote:
>> Add a more specific board compatible entry for all of the SOCFPGA
>> Cyclone 5 based boards.
>>

[snip]

>>  / {
>>  	model = "Altera SOCFPGA Cyclone V SoC Development Kit";
>> -	compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> +	compatible = "altr,socdk", "altr,socfpga-cyclone5", "altr,socfpga";
> 
> This looks a little too generic, what if there's another dk with another
> SoC down the road?
> 

Right...I'll change it to "altr,socfpga-cyclone5-socdk",

>>  	chosen {
>>  		bootargs = "earlyprintk";
>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> index 02e22f5..c5623a7 100644
>> --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> @@ -19,7 +19,7 @@
>>  
>>  / {
>>  	model = "Terasic SoCkit";
>> -	compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> +	compatible = "terasic,sockit", "altr,socfpga-cyclone5", "altr,socfpga";
> 
> Same thing here, this seems a bit on the generic side.
> 

perhaps "terasic,socfpga-cyclone5-sockit" ?

Thanks for reviewing.

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

* [PATCHv2] ARM: dts: socfpga: add specific compatible strings for boards
From: Dinh Nguyen @ 2016-11-08 20:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161101222352.GA6328@localhost>



On 11/01/2016 05:23 PM, Olof Johansson wrote:
> On Tue, Nov 01, 2016 at 03:56:52PM -0500, Dinh Nguyen wrote:
>> Add a more specific board compatible entry for all of the SOCFPGA
>> Cyclone 5 based boards.
>>

[snip]

>>  / {
>>  	model = "Altera SOCFPGA Cyclone V SoC Development Kit";
>> -	compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> +	compatible = "altr,socdk", "altr,socfpga-cyclone5", "altr,socfpga";
> 
> This looks a little too generic, what if there's another dk with another
> SoC down the road?
> 

Right...I'll change it to "altr,socfpga-cyclone5-socdk",

>>  	chosen {
>>  		bootargs = "earlyprintk";
>> diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> index 02e22f5..c5623a7 100644
>> --- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> +++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
>> @@ -19,7 +19,7 @@
>>  
>>  / {
>>  	model = "Terasic SoCkit";
>> -	compatible = "altr,socfpga-cyclone5", "altr,socfpga";
>> +	compatible = "terasic,sockit", "altr,socfpga-cyclone5", "altr,socfpga";
> 
> Same thing here, this seems a bit on the generic side.
> 

perhaps "terasic,socfpga-cyclone5-sockit" ?

Thanks for reviewing.

Dinh

^ permalink raw reply

* [Buildroot] [PATCH v4 04/22] toolchain-external: introduce toolchain-external-package
From: Arnout Vandecappelle @ 2016-11-08 20:53 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <20161108163847.76ce7e6f@free-electrons.com>



On 08-11-16 16:38, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 8 Nov 2016 13:30:16 +0100, Arnout Vandecappelle wrote:
> 
>>  That would be useful if such a toolchain uncovers problems that we can fix in
>> Buildroot. If all it does is to add autobuilder exceptions, there isn't much
>> point... Well, the other thing we can do is to add *_AT_LEAST_NNN options,
>> that's true.
> 
> Yes, that's what I'm thinking of: it allows us to realize when a
> package needs gcc 4.7 or 4.8, or when some fairly recent kernel headers
> are needed, for example. If all we test is gcc 4.9 with 4.0 kernel
> headers, then all our users that are stuck with 4.7 (or older
> toolchains) with 3.0 kernel headers will have a really bad experience.

 For things which we don't test in the autobuilders, I think we should retire
the _AT_LEAST_ option. What is the use of having an option that tells you which
packages will work with gcc 4.6, if we actually have no idea if those packages
will work with gcc 4.6? So indeed, removing the blackfin toolchain would mean
removing gcc 4.3, 4.4, 4.5, 4.6 and 4.7 (the lowest option is actually useless
because it is always set).

 Anyway, the Blackfin toolchain is now the only one with gcc < 4.7, but this
toolchain wasn't really great for testing gcc compatibility, because many
packages are already excluded due to its various other arch limitations.

 Ideally we should generate a gcc 4.5 - headers 3.0 - glibc 2.19 based toolchain
for a few architectures. We can use 2015.02 for that. uClibc is not so
interesting because older versions can break in so many ways, and because less
packages are tested with it.

> 
>>  Those autobuilder exceptions for ct-ng, what kind of errors are they for? Not
>> something covered by *_AT_LEAST? Like bugs in libc, for which we don't have
>> _AT_LEAST?
> 
> Looking quickly at the autobuild-run exceptions, we have:
> 
>  - Toolchains with gcc affected by various PRs

 But that should be handled with _AT_LEAST, no?

> 
>  - Toolchains with too old eglibc (2.18)
> 
>  - Toolchains that fail to build this or that package, with no
>    documented reason (might be explained in the commit log, though).

 Yeah, I've taken a look at the commit logs and it's usually because of too old
uclibc.

 I also noticed that the majority of the exceptions are for toolchains that are
no longer in toolchain-configs.csv...

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply

* Re: Error migrating VM to secondary host using COLO replication
From: Sadi @ 2016-11-08 20:53 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: Wen Congyang, xen-devel
In-Reply-To: <20161108195721.GB20807@char.us.oracle.com>


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

Hi,

Apparently vif2.0-emu was already binded with br0 when "brctl addif br0
vif2.0-emu" failed.

root@colob-HP-Compaq-6005-Pro-MT-PC:~# brctl addif br0 vif2.0-emu
device vif2.0-emu is already a member of a bridge; can't enslave it to
bridge br0.
root@colob-HP-Compaq-6005-Pro-MT-PC:~# brctl show
bridge name     bridge id               STP enabled     interfaces
br0             8000.001a3fc46255       no              eth0
                                                        vif2.0
                                                        vif2.0-emu
br1             8000.000000000000       no

About the iptables, it seems like SECCOLO target can't be recognised.

root@colob-HP-Compaq-6005-Pro-MT-PC:~# iptables -t mangle -D PREROUTING -m
physdev --physdev-in vif2.0-emu -j SECCOLO --index 1
iptables: No chain/target/match by that name.

Here is my active modules matching colo:

root@colob-HP-Compaq-6005-Pro-MT-PC:~# lsmod | grep -i colo
xt_SECCOLO             16384  1
nf_conntrack_colo      16384  2 xt_SECCOLO
x_tables               36864  8
xt_physdev,ip6table_mangle,ip_tables,xt_SECCOLO,xt_tcpudp,iptable_filter,iptable_mangle,ip6_tables
nf_conntrack          106496  4
xt_SECCOLO,nf_nat,nf_conntrack_colo,nf_conntrack_ipv4

So i was looking in the iptables and this really looks like the source of
the problem.

Sadi.

On Tue, Nov 8, 2016 at 5:57 PM, Konrad Rzeszutek Wilk <
konrad.wilk@oracle.com> wrote:

> > entered forwarding state
> > Nov  7 18:10:30 colob NetworkManager[907]: <info>  (vif2.0-emu): enslaved
> > to br0
> > Nov  7 18:10:30 colob root: /etc/xen/scripts/colo-proxy-setup: brctl
> addif
> > br0 vif2.0-emu failed
>
>
> How come this failed?
>
> > Nov  7 18:10:30 colob root: /etc/xen/scripts/colo-proxy-setup: iptables
> -t
> > mangle -D PREROUTING -m physdev --physdev-in vif2.0-emu -j SECCOLO
> --index
> > 1 failed
>
> Ah b/c of this. Are there any errors of what exactly failed?
>



-- 
Sadi.

[-- Attachment #1.2: Type: text/html, Size: 2962 bytes --]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* [U-Boot] Pull request: u-boot-net.git master
From: Tom Rini @ 2016-11-08 20:52 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <20161107172938.56086FEB38@linux-xvxi.amer.corp.natinst.com>

On Mon, Nov 07, 2016 at 11:29:02AM -0600, Joe Hershberger wrote:

> Hi Tom,
> 
> The following changes since commit 4b6035da482cccda06aeb419634f99937c9fc783:
> 
>   mx6sabresd: Make Ethernet functional again (2016-11-06 06:59:27 -0500)
> 
> are available in the git repository at:
> 
>   git://git.denx.de/u-boot-net.git master
> 
> for you to fetch changes up to aa555fe9f07a21b3bbdab15aea594f3869e5ab22:
> 
>   net: use random ethernet address if invalid and not zero (2016-11-07 11:28:16 -0600)
> 

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161108/b9f649f0/attachment.sig>

^ permalink raw reply

* [PATCH] nvme-rdma: stop and free io queues on connect failure
From: Steve Wise @ 2016-11-08 20:52 UTC (permalink / raw)

In-Reply-To: <20161108200724.3F817E09E2@smtp.ogc.us>

Hey Sagi,

With v4.9-rc4 + your nvmf-4.9-rc branch rebased on rc4, and this fix below on
the host side, and your target side patch from:

http://lists.infradead.org/pipermail/linux-nvme/2016-November/007109.html

I've been up and running my keep-alive timeout/reconnect under load test for a
while now.  34 iterations of the fio.sh script I sent out in the above thread.
I'll keep testing, but I think we're getting close...

Steve.

> -----Original Message-----
> From: Linux-nvme [mailto:linux-nvme-bounces at lists.infradead.org] On Behalf Of
> Steve Wise
> Sent: Tuesday, November 08, 2016 11:16 AM
> To: sagi at grimberg.me
> Cc: hch at lst.de; linux-nvme at lists.infradead.org
> Subject: [PATCH] nvme-rdma: stop and free io queues on connect failure
> 
> While testing nvme-rdma with the spdk nvmf target over iw_cxgb4, I
> configured the target (mistakenly) to generate an error creating the
> NVMF IO queues.  This resulted a "Invalid SQE Parameter" error sent back
> to the host on the first IO queue connect:
> 
> [ 9610.928182] nvme nvme1: queue_size 128 > ctrl maxcmd 120, clamping down
> [ 9610.938745] nvme nvme1: creating 32 I/O queues.
> 
> So nvmf_connect_io_queue() returns an error to
> nvmf_connect_io_queue() / nvmf_connect_io_queues(), and that
> is returned to nvme_rdma_create_io_queues().  In the error path,
> nvmf_rdma_create_io_queues() frees the queue tagset memory _before_
> stopping and freeing the IB queues, which causes yet another
> touch-after-free crash due to SQ CQEs being flushed after the ib_cqe
> structs pointed-to by the flushed WRs have been freed (since they are
> part of the nvme_rdma_request struct).
> 
> The fix is to stop and free the queues in nvmf_connect_io_queues()
> if there is an error connecting any of the queues.
> 
> Signed-off-by: Steve Wise <swise at opengridcomputing.com>
> ---
>  drivers/nvme/host/rdma.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index af1c114..0504f39 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -624,11 +624,18 @@ static int nvme_rdma_connect_io_queues(struct
> nvme_rdma_ctrl *ctrl)
> 
>  	for (i = 1; i < ctrl->queue_count; i++) {
>  		ret = nvmf_connect_io_queue(&ctrl->ctrl, i);
> -		if (ret)
> -			break;
> +		if (ret) {
> +			dev_info(ctrl->ctrl.device,
> +				"failed to connect i/o queue: %d\n", ret);
> +			goto out_free_queues;
> +		}
>  		set_bit(NVME_RDMA_Q_LIVE, &ctrl->queues[i].flags);
>  	}
> 
> +	return 0;
> +
> +out_free_queues:
> +	nvme_rdma_free_io_queues(ctrl);
>  	return ret;
>  }
> 
> --
> 2.7.0
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply

* Re: [PATCH v3] PM / devfreq: Restart previous governor if new governor fails to start
From: Saravana Kannan @ 2016-11-08 20:52 UTC (permalink / raw)
  To: Chanwoo Choi
  Cc: Rafael J. Wysocki, MyungJoo Ham, Kyungmin Park, linux-pm,
	linux-kernel
In-Reply-To: <5821948B.2010907@samsung.com>

On 11/08/2016 01:02 AM, Chanwoo Choi wrote:
> Hi,
>
> On 2016년 11월 08일 03:57, Saravana Kannan wrote:
>> On 10/26/2016 05:06 PM, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> On 2016년 10월 27일 04:17, Saravana Kannan wrote:
>>>> If the new governor fails to start, switch back to old governor so that the
>>>> devfreq state is not left in some weird limbo.
>>>>
>>>> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
>>>> ---
>>>> * Fix NULL deref for real this time.
>>>> * Addressed some style preferences.
>>>>
>>>>    drivers/devfreq/devfreq.c | 13 +++++++++++--
>>>>    1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>> index bf3ea76..77c41a5 100644
>>>> --- a/drivers/devfreq/devfreq.c
>>>> +++ b/drivers/devfreq/devfreq.c
>>>> @@ -919,7 +919,7 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>>>>        struct devfreq *df = to_devfreq(dev);
>>>>        int ret;
>>>>        char str_governor[DEVFREQ_NAME_LEN + 1];
>>>> -    struct devfreq_governor *governor;
>>>> +    const struct devfreq_governor *governor, *prev_governor;
>>>>
>>>>        ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
>>>>        if (ret != 1)
>>>> @@ -944,12 +944,21 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr,
>>>>                goto out;
>>>>            }
>>>>        }
>>>> +    prev_governor = df->governor;
>>>>        df->governor = governor;
>>>>        strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN);
>>>>        ret = df->governor->event_handler(df, DEVFREQ_GOV_START, NULL);
>>>> -    if (ret)
>>>> +    if (ret) {
>>>>            dev_warn(dev, "%s: Governor %s not started(%d)\n",
>>>>                 __func__, df->governor->name, ret);
>>>> +        if (prev_governor) {
>>>
>>> I think that prev_governor is always set. You don't need to check it.
>>> Why do check it?
>>
>> Not true. Even higher up in this function, we check if df->governor != NULL. Simple example would be that the default governor passed in while adding the device isn't compiled in.
>
> I don't understand. If device is not registered by devfreq_add_device(),
> you don't change the governor by using governor_store().
>
> If you can change the governor through governor_store(),
> it means that the devfreq instance was added without any problem.
> The added devfreq instance must have the sepecific governor
> on df->governor variable.
>
> So, I think that df->governor is always set and then prev_governor is always set.

Read the code more closely. add_device doesn't (and shouldn't) fail if 
the governor isn't present. After that, the governor could be changed 
from user space.

-Saravana

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: Configure RAM size on iMX53 board
From: Jose Luis Zabalza @ 2016-11-08 20:51 UTC (permalink / raw)
  To: barebox
In-Reply-To: <20161108075317.j4xkf7zpkdfyt5y3@pengutronix.de>

> So you have 512MiB on each chip select, so I assume that on the 512MiB
> board variants CS1 is not equipped.

Yes, it is.

>In that case you can in lowlevel.c
> test if you find SDRAM on CS1 and if not, disable the chip select
> completely in the SDRAM controller.

OK. But how ? I enable CS0 and CS1 on DCD table. Is there any way to
tell barebox not to use CS1 ?

> I am not sure how you can detect if there's SDRAM on CS1. I've seen
> situations in which the board just hangs if you access non existent RAM
> areas.

I have tried it, but I have not be able to implement a code for
autodetect. If the code write or read a value on a position without
physical  chip, the microcontroller hangs. ????

But it's not a problem. A solution is configure both CS and MMU. If
bootloader don't access to high positions, there is not problem. After
I set a environment variable with memory size and the mem kernel
parameter (or ATAG) does the rest.

So, I have to make sure Barebox don't reach CS1 memory positions. That is all.

Thanks for your help. See you later.


-- 

jlz.3008  a t  gmail.com
Linux Counter 172551
https://linuxcounter.net/cert/172551.png

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply

* Re: [PATCH v2 23/23] staging: iio: tsl2583: move out of staging
From: Jonathan Cameron @ 2016-11-08 20:50 UTC (permalink / raw)
  To: Brian Masney, linux-iio
  Cc: devel, gregkh, lars, pmeerw, knaack.h, linux-kernel, Jon.Brenner
In-Reply-To: <1478600208-21153-24-git-send-email-masneyb@onstation.org>

On 08/11/16 10:16, Brian Masney wrote:
> Move tsl2580, tsl2581, tsl2583 driver out of staging into mainline.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
A few minor bits and bobs inline - might need to push one or two into relevant
earlier patches, but was easier to just review the whole code in here as
I'm out of time for tonight.

Jonathan
> ---
>  .../ABI/testing/sysfs-bus-iio-light-tsl2583        |  20 +
>  drivers/iio/light/Kconfig                          |   7 +
>  drivers/iio/light/Makefile                         |   1 +
>  drivers/iio/light/tsl2583.c                        | 916 +++++++++++++++++++++
>  .../light/sysfs-bus-iio-light-tsl2583              |  20 -
>  drivers/staging/iio/light/Kconfig                  |   7 -
>  drivers/staging/iio/light/Makefile                 |   1 -
>  drivers/staging/iio/light/tsl2583.c                | 916 ---------------------
>  8 files changed, 944 insertions(+), 944 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-tsl2583
>  create mode 100644 drivers/iio/light/tsl2583.c
>  delete mode 100644 drivers/staging/iio/Documentation/light/sysfs-bus-iio-light-tsl2583
>  delete mode 100644 drivers/staging/iio/light/tsl2583.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-tsl2583 b/Documentation/ABI/testing/sysfs-bus-iio-light-tsl2583
> new file mode 100644
> index 0000000..a2e1996
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-light-tsl2583
> @@ -0,0 +1,20 @@
> +What:		/sys/bus/iio/devices/device[n]/in_illuminance_calibrate
> +KernelVersion:	2.6.37
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This property causes an internal calibration of the als gain trim
> +		value which is later used in calculating illuminance in lux.
> +
> +What:		/sys/bus/iio/devices/device[n]/in_illuminance_lux_table
> +KernelVersion:	2.6.37
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This property gets/sets the table of coefficients
> +		used in calculating illuminance in lux.
> +
> +What:		/sys/bus/iio/devices/device[n]/in_illuminance_input_target
> +KernelVersion:	2.6.37
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		This property is the known externally illuminance (in lux).
> +		It is used in the process of calibrating the device accuracy.
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index d011720..298ea50 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -338,6 +338,13 @@ config SENSORS_TSL2563
>  	 This driver can also be built as a module.  If so, the module
>  	 will be called tsl2563.
>  
> +config TSL2583
> +	tristate "TAOS TSL2580, TSL2581 and TSL2583 light-to-digital converters"
> +	depends on I2C
> +	help
> +	 Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
> +	 Access ALS data via iio, sysfs.
> +
>  config TSL4531
>  	tristate "TAOS TSL4531 ambient light sensors"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index 15f24c5..4de5200 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_SI1145)		+= si1145.o
>  obj-$(CONFIG_STK3310)          += stk3310.o
>  obj-$(CONFIG_TCS3414)		+= tcs3414.o
>  obj-$(CONFIG_TCS3472)		+= tcs3472.o
> +obj-$(CONFIG_TSL2583)		+= tsl2583.o
>  obj-$(CONFIG_TSL4531)		+= tsl4531.o
>  obj-$(CONFIG_US5182D)		+= us5182d.o
>  obj-$(CONFIG_VCNL4000)		+= vcnl4000.o
> diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c
> new file mode 100644
> index 0000000..5cb4ae3
> --- /dev/null
> +++ b/drivers/iio/light/tsl2583.c
> @@ -0,0 +1,916 @@
> +/*
> + * Device driver for monitoring ambient light intensity (lux)
> + * within the TAOS tsl258x family of devices (tsl2580, tsl2581).
If we are going to have a list it should be complete.  tsl2583 as well.
> + *
> + * Copyright (c) 2011, TAOS Corporation.
> + * Copyright (c) 2016 Brian Masney <masneyb@onstation.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/errno.h>
> +#include <linux/delay.h>
> +#include <linux/string.h>
> +#include <linux/mutex.h>
> +#include <linux/unistd.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +/* Device Registers and Masks */
> +#define TSL2583_CNTRL			0x00
> +#define TSL2583_ALS_TIME		0X01
> +#define TSL2583_INTERRUPT		0x02
> +#define TSL2583_GAIN			0x07
> +#define TSL2583_REVID			0x11
> +#define TSL2583_CHIPID			0x12
> +#define TSL2583_ALS_CHAN0LO		0x14
> +#define TSL2583_ALS_CHAN0HI		0x15
> +#define TSL2583_ALS_CHAN1LO		0x16
> +#define TSL2583_ALS_CHAN1HI		0x17
> +#define TSL2583_TMR_LO			0x18
> +#define TSL2583_TMR_HI			0x19
> +
> +/* tsl2583 cmd reg masks */
> +#define TSL2583_CMD_REG			0x80
> +#define TSL2583_CMD_SPL_FN		0x60
> +#define TSL2583_CMD_ALS_INT_CLR		0x01
> +
> +/* tsl2583 cntrl reg masks */
> +#define TSL2583_CNTL_ADC_ENBL		0x02
> +#define TSL2583_CNTL_PWR_OFF		0x00
> +#define TSL2583_CNTL_PWR_ON		0x01
> +
> +/* tsl2583 status reg masks */
> +#define TSL2583_STA_ADC_VALID		0x01
> +#define TSL2583_STA_ADC_INTR		0x10
> +
> +/* Lux calculation constants */
> +#define	TSL2583_LUX_CALC_OVER_FLOW	65535
> +
> +#define TSL2583_INTERRUPT_DISABLED	0x00
> +
> +#define TSL2583_CHIP_ID			0x90
> +#define TSL2583_CHIP_ID_MASK		0xf0
> +
> +/* Per-device data */
> +struct tsl2583_als_info {
> +	u16 als_ch0;
> +	u16 als_ch1;
> +	u16 lux;
> +};
> +
> +struct tsl2583_lux {
> +	unsigned int ratio;
> +	unsigned int ch0;
> +	unsigned int ch1;
> +};
> +
> +static const struct tsl2583_lux tsl2583_default_lux[] = {
> +	{  9830,  8520, 15729 },
> +	{ 12452, 10807, 23344 },
> +	{ 14746,  6383, 11705 },
> +	{ 17695,  4063,  6554 },
> +	{     0,     0,     0 }  /* Termination segment */
> +};
> +
> +struct tsl2583_settings {
> +	int als_time;
> +	int als_gain;
> +	int als_gain_trim;
> +	int als_cal_target;
> +
> +	/*
> +	 * This structure is intentionally large to accommodate updates via
> +	 * sysfs. Sized to 11 = max 10 segments + 1 termination segment.
> +	 * Assumption is that one and only one type of glass used.
> +	 */
> +	struct tsl2583_lux als_device_lux[11];
> +};
> +
> +struct tsl2583_chip {
> +	struct mutex als_mutex;
> +	struct i2c_client *client;
> +	struct tsl2583_als_info als_cur_info;
> +	struct tsl2583_settings als_settings;
> +	int als_time_scale;
> +	int als_saturation;
> +	bool suspended;
> +};
> +
> +struct gainadj {
> +	s16 ch0;
> +	s16 ch1;
> +	s16 mean;
> +};
> +
> +/* Index = (0 - 3) Used to validate the gain selection index */
> +static const struct gainadj gainadj[] = {
> +	{ 1, 1, 1 },
> +	{ 8, 8, 8 },
> +	{ 16, 16, 16 },
> +	{ 107, 115, 111 }
> +};
> +
> +/*
> + * Provides initial operational parameter defaults.
> + * These defaults may be changed through the device's sysfs files.
> + */
> +static void tsl2583_defaults(struct tsl2583_chip *chip)
> +{
> +	/*
> +	 * The integration time must be a multiple of 50ms and within the
> +	 * range [50, 600] ms.
> +	 */
> +	chip->als_settings.als_time = 100;
> +
> +	/*
> +	 * This is an index into the gainadj table. Assume clear glass as the
> +	 * default.
> +	 */
> +	chip->als_settings.als_gain = 0;
> +
> +	/* Default gain trim to account for aperture effects */
> +	chip->als_settings.als_gain_trim = 1000;
> +
> +	/* Known external ALS reading used for calibration */
> +	chip->als_settings.als_cal_target = 130;
> +
> +	/* Default lux table. */
> +	memcpy(chip->als_settings.als_device_lux, tsl2583_default_lux,
> +	       sizeof(tsl2583_default_lux));
> +}
> +
> +/*
> + * Reads and calculates current lux value.
> + * The raw ch0 and ch1 values of the ambient light sensed in the last
> + * integration cycle are read from the device.
> + * Time scale factor array values are adjusted based on the integration time.
> + * The raw values are multiplied by a scale factor, and device gain is obtained
> + * using gain index. Limit checks are done next, then the ratio of a multiple
> + * of ch1 value, to the ch0 value, is calculated. The array als_device_lux[]
> + * declared above is then scanned to find the first ratio value that is just
> + * above the ratio we just calculated. The ch0 and ch1 multiplier constants in
> + * the array are then used along with the time scale factor array values, to
> + * calculate the lux.
> + */
> +static int tsl2583_get_lux(struct iio_dev *indio_dev)
> +{
> +	u16 ch0, ch1; /* separated ch0/ch1 data from device */
> +	u32 lux; /* raw lux calculated from device data */
> +	u64 lux64;
> +	u32 ratio;
> +	u8 buf[5];
> +	struct tsl2583_lux *p;
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int i, ret;
> +
> +	ret = i2c_smbus_read_byte_data(chip->client, TSL2583_CMD_REG);
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev, "%s failed to read CMD_REG register\n",
> +			__func__);
> +		goto done;
> +	}
> +
> +	/* is data new & valid */
> +	if (!(ret & TSL2583_STA_ADC_INTR)) {
> +		dev_err(&chip->client->dev, "%s: data not valid; returning last value\n",
> +			__func__);
> +		ret = chip->als_cur_info.lux; /* return LAST VALUE */
> +		goto done;
> +	}
> +
> +	for (i = 0; i < 4; i++) {
> +		int reg = TSL2583_CMD_REG | (TSL2583_ALS_CHAN0LO + i);
> +
> +		ret = i2c_smbus_read_byte_data(chip->client, reg);
> +		if (ret < 0) {
> +			dev_err(&chip->client->dev, "%s failed to read register %x\n",
> +				__func__, reg);
> +			goto done;
> +		}
> +		buf[i] = ret;
> +	}
> +
> +	/*
> +	 * clear status, really interrupt status (interrupts are off), but
> +	 * we use the bit anyway - don't forget 0x80 - this is a command
? Feel free to figure out which bits of that are actually useful and delete
the rest ;)
> +	 */
> +	ret = i2c_smbus_write_byte(chip->client,
> +				   (TSL2583_CMD_REG | TSL2583_CMD_SPL_FN |
> +				    TSL2583_CMD_ALS_INT_CLR));
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev, "%s failed to clear the interrupt bit\n",
> +			__func__);
> +		goto done; /* have no data, so return failure */
> +	}
> +
> +	/* extract ALS/lux data */
> +	ch0 = le16_to_cpup((const __le16 *)&buf[0]);
> +	ch1 = le16_to_cpup((const __le16 *)&buf[2]);
> +
> +	chip->als_cur_info.als_ch0 = ch0;
> +	chip->als_cur_info.als_ch1 = ch1;
> +
> +	if ((ch0 >= chip->als_saturation) || (ch1 >= chip->als_saturation))
> +		goto return_max;
> +
> +	if (!ch0) {
> +		/* have no data, so return 0 */
> +		ret = 0;
> +		chip->als_cur_info.lux = 0;
> +		goto done;
> +	}
> +
> +	/* calculate ratio */
> +	ratio = (ch1 << 15) / ch0;
> +
> +	/* convert to unscaled lux using the pointer to the table */
> +	for (p = (struct tsl2583_lux *)chip->als_settings.als_device_lux;
> +	     p->ratio != 0 && p->ratio < ratio; p++)
> +		;
> +
> +	if (p->ratio == 0) {
> +		lux = 0;
> +	} else {
> +		u32 ch0lux, ch1lux;
> +
> +		ch0lux = ((ch0 * p->ch0) +
> +			  (gainadj[chip->als_settings.als_gain].ch0 >> 1))
> +			 / gainadj[chip->als_settings.als_gain].ch0;
> +		ch1lux = ((ch1 * p->ch1) +
> +			  (gainadj[chip->als_settings.als_gain].ch1 >> 1))
> +			 / gainadj[chip->als_settings.als_gain].ch1;
> +
> +		/* note: lux is 31 bit max at this point */
> +		if (ch1lux > ch0lux) {
> +			dev_dbg(&chip->client->dev, "%s: No Data - Returning 0\n",
> +				__func__);
> +			ret = 0;
> +			chip->als_cur_info.lux = 0;
> +			goto done;
> +		}
> +
> +		lux = ch0lux - ch1lux;
> +	}
> +
> +	/* adjust for active time scale */
> +	if (chip->als_time_scale == 0)
> +		lux = 0;
> +	else
> +		lux = (lux + (chip->als_time_scale >> 1)) /
> +			chip->als_time_scale;
> +
> +	/*
> +	 * Adjust for active gain scale.
> +	 * The tsl2583_default_lux tables above have a factor of 8192 built in,
> +	 * so we need to shift right.
> +	 * User-specified gain provides a multiplier.
> +	 * Apply user-specified gain before shifting right to retain precision.
> +	 * Use 64 bits to avoid overflow on multiplication.
> +	 * Then go back to 32 bits before division to avoid using div_u64().
> +	 */
> +	lux64 = lux;
> +	lux64 = lux64 * chip->als_settings.als_gain_trim;
> +	lux64 >>= 13;
> +	lux = lux64;
> +	lux = (lux + 500) / 1000;
> +
> +	if (lux > TSL2583_LUX_CALC_OVER_FLOW) { /* check for overflow */
> +return_max:
> +		lux = TSL2583_LUX_CALC_OVER_FLOW;
> +	}
> +
> +	/* Update the structure with the latest VALID lux. */
> +	chip->als_cur_info.lux = lux;
> +	ret = lux;
> +
> +done:
> +	return ret;
> +}
> +
> +/*
> + * Obtain single reading and calculate the als_gain_trim (later used
> + * to derive actual lux).
> + * Return updated gain_trim value.
> + */
> +static int tsl2583_als_calibrate(struct iio_dev *indio_dev)
> +{
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	unsigned int gain_trim_val;
> +	int ret;
> +	int lux_val;
> +
> +	ret = i2c_smbus_read_byte_data(chip->client,
> +				       TSL2583_CMD_REG | TSL2583_CNTRL);
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev,
> +			"%s failed to read from the CNTRL register\n",
> +			__func__);
> +		return ret;
> +	}
> +
> +	if ((ret & (TSL2583_CNTL_ADC_ENBL | TSL2583_CNTL_PWR_ON))
> +			!= (TSL2583_CNTL_ADC_ENBL | TSL2583_CNTL_PWR_ON)) {
> +		dev_err(&chip->client->dev,
> +			"%s failed: device not powered on with ADC enabled\n",
The negative is confusing in this error.
Device not powered on and / or ADC not enabled (I think)
> +			__func__);
> +		return -EINVAL;
> +	} else if ((ret & TSL2583_STA_ADC_VALID) != TSL2583_STA_ADC_VALID) {
> +		dev_err(&chip->client->dev,
> +			"%s failed: STATUS - ADC not valid.\n", __func__);
ADC reading or ADC not in valid state?
> +		return -ENODATA;
> +	}
> +
> +	lux_val = tsl2583_get_lux(indio_dev);
> +	if (lux_val < 0) {
> +		dev_err(&chip->client->dev, "%s failed to get lux\n", __func__);
> +		return lux_val;
> +	}
> +
> +	gain_trim_val = (unsigned int)(((chip->als_settings.als_cal_target)
> +			* chip->als_settings.als_gain_trim) / lux_val);
> +	if ((gain_trim_val < 250) || (gain_trim_val > 4000)) {
> +		dev_err(&chip->client->dev,
> +			"%s failed: trim_val of %d is not within the range [250, 4000]\n",
> +			__func__, gain_trim_val);
> +		return -ENODATA;
> +	}
> +
> +	chip->als_settings.als_gain_trim = (int)gain_trim_val;
> +
> +	return 0;
> +}
> +
> +static int tsl2583_set_als_time(struct tsl2583_chip *chip)
> +{
> +	int als_count, als_time, ret;
> +	u8 val;
> +
> +	/* determine als integration register */
> +	als_count = (chip->als_settings.als_time * 100 + 135) / 270;
> +	if (!als_count)
> +		als_count = 1; /* ensure at least one cycle */
> +
> +	/* convert back to time (encompasses overrides) */
> +	als_time = (als_count * 27 + 5) / 10;
> +
> +	val = 256 - als_count;
> +	ret = i2c_smbus_write_byte_data(chip->client,
> +					TSL2583_CMD_REG | TSL2583_ALS_TIME,
> +					val);
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev, "%s failed to set the als time to %d\n",
> +			__func__, val);
> +		return ret;
> +	}
> +
> +	/* set chip struct re scaling and saturation */
> +	chip->als_saturation = als_count * 922; /* 90% of full scale */
> +	chip->als_time_scale = (als_time + 25) / 50;
> +
> +	return ret;
> +}
> +
> +static int tsl2583_set_als_gain(struct tsl2583_chip *chip)
> +{
> +	int ret;
> +
> +	/* Set the gain based on als_settings struct */
> +	ret = i2c_smbus_write_byte_data(chip->client,
> +					TSL2583_CMD_REG | TSL2583_GAIN,
> +					chip->als_settings.als_gain);
> +	if (ret < 0)
> +		dev_err(&chip->client->dev, "%s failed to set the gain to %d\n",
> +			__func__, chip->als_settings.als_gain);
> +
> +	return ret;
> +}
> +
> +static int tsl2583_set_power_state(struct tsl2583_chip *chip, u8 state)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(chip->client,
> +					TSL2583_CMD_REG | TSL2583_CNTRL, state);
> +	if (ret < 0)
> +		dev_err(&chip->client->dev, "%s failed to set the power state to %d\n",
> +			__func__, state);
> +
> +	return ret;
> +}
> +
> +/*
> + * Turn the device on.
> + * Configuration must be set before calling this function.
> + */
> +static int tsl2583_chip_init_and_power_on(struct iio_dev *indio_dev)
> +{
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret;
> +
> +	/* Power on the device; ADC off. */
> +	ret = tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_ON);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = i2c_smbus_write_byte_data(chip->client,
> +					TSL2583_CMD_REG | TSL2583_INTERRUPT,
> +					TSL2583_INTERRUPT_DISABLED);
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev, "%s failed to disable interrupts\n",
> +			__func__);
> +		return ret;
> +	}
> +
> +	ret = tsl2583_set_als_time(chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = tsl2583_set_als_gain(chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	usleep_range(3000, 3500);
> +
> +	ret = tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_ON |
> +					    TSL2583_CNTL_ADC_ENBL);
> +	if (ret < 0)
> +		return ret;
> +
> +	chip->suspended = false;
> +
> +	return ret;
> +}
> +
> +/* Sysfs Interface Functions */
> +
> +static ssize_t in_illuminance_input_target_show(struct device *dev,
> +						struct device_attribute *attr,
> +						char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret;
> +
> +	mutex_lock(&chip->als_mutex);
> +	ret = sprintf(buf, "%d\n", chip->als_settings.als_cal_target);
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return ret;
> +}
> +
> +static ssize_t in_illuminance_input_target_store(struct device *dev,
> +						 struct device_attribute *attr,
> +						 const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int value;
> +
> +	if (kstrtoint(buf, 0, &value) || !value)
> +		return -EINVAL;
> +
> +	mutex_lock(&chip->als_mutex);
> +	chip->als_settings.als_cal_target = value;
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return len;
> +}
> +
> +static ssize_t in_illuminance_calibrate_store(struct device *dev,
> +					      struct device_attribute *attr,
> +					      const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int value, ret;
> +
> +	if (kstrtoint(buf, 0, &value) || value != 1)
> +		return -EINVAL;
> +
> +	mutex_lock(&chip->als_mutex);
> +
> +	if (chip->suspended) {
> +		ret = -EBUSY;
> +		goto done;
> +	}
> +
> +	ret = tsl2583_als_calibrate(indio_dev);
> +	if (ret < 0)
> +		goto done;
> +
> +	ret = len;
> +done:
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return ret;
> +}
> +
> +static ssize_t in_illuminance_lux_table_show(struct device *dev,
> +					     struct device_attribute *attr,
> +					     char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	unsigned int i;
> +	int offset = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(chip->als_settings.als_device_lux); i++) {
> +		offset += sprintf(buf + offset, "%u,%u,%u,",
> +				  chip->als_settings.als_device_lux[i].ratio,
> +				  chip->als_settings.als_device_lux[i].ch0,
> +				  chip->als_settings.als_device_lux[i].ch1);
> +		if (chip->als_settings.als_device_lux[i].ratio == 0) {
> +			/*
> +			 * We just printed the first "0" entry.
> +			 * Now get rid of the extra "," and break.
> +			 */
> +			offset--;
> +			break;
> +		}
> +	}
> +
> +	offset += sprintf(buf + offset, "\n");
> +
> +	return offset;
> +}
> +
> +static ssize_t in_illuminance_lux_table_store(struct device *dev,
> +					      struct device_attribute *attr,
> +					      const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	const unsigned int max_ints =
> +		ARRAY_SIZE(chip->als_settings.als_device_lux) * 3;
> +	int value[max_ints];
> +	int ret = -EINVAL;
> +	unsigned int n;
> +
> +	mutex_lock(&chip->als_mutex);
> +
> +	get_options(buf, ARRAY_SIZE(value), value);
> +
> +	/*
> +	 * We now have an array of ints starting at value[1], and
> +	 * enumerated by value[0].
> +	 * We expect each group of three ints is one table entry,
> +	 * and the last table entry is all 0.
> +	 */
> +	n = value[0];
> +	if ((n % 3) || n < 6 || n > max_ints) {
> +		dev_err(dev,
> +			"%s: The number of entries in the lux table must be a multiple of 3 and within the range [6, %d]",
> +			__func__, max_ints);
> +		goto done;
> +	}
> +	if ((value[n - 2] | value[n - 1] | value[n]) != 0) {
> +		dev_err(dev, "%s: The last 3 entries in the lux table must be zeros.\n",
> +			__func__);
> +		goto done;
> +	}
> +
> +	/* Zero out the table */
> +	memset(chip->als_settings.als_device_lux, 0,
> +	       sizeof(chip->als_settings.als_device_lux));
Would be more elegant to just zero the remaining entries after the memcpy.
Doesn't really matter though ;)
> +	memcpy(chip->als_settings.als_device_lux, &value[1],
> +	       value[0] * sizeof(unsigned int));
> +
> +	ret = len;
> +
> +done:
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return ret;
> +}
> +
> +static IIO_CONST_ATTR(in_illuminance_calibscale_available, "1 8 16 111");
> +static IIO_CONST_ATTR(in_illuminance_integration_time_available,
> +		      "0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
> +static IIO_DEVICE_ATTR_RW(in_illuminance_input_target, 0);
> +static IIO_DEVICE_ATTR_WO(in_illuminance_calibrate, 0);
> +static IIO_DEVICE_ATTR_RW(in_illuminance_lux_table, 0);
> +
> +static struct attribute *sysfs_attrs_ctrl[] = {
> +	&iio_const_attr_in_illuminance_calibscale_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
> +	&iio_dev_attr_in_illuminance_input_target.dev_attr.attr,
> +	&iio_dev_attr_in_illuminance_calibrate.dev_attr.attr,
> +	&iio_dev_attr_in_illuminance_lux_table.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group tsl2583_attribute_group = {
> +	.attrs = sysfs_attrs_ctrl,
> +};
> +
> +static const struct iio_chan_spec tsl2583_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_IR,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +	},
> +	{
> +		.type = IIO_LIGHT,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_LIGHT_BOTH,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +	},
> +	{
> +		.type = IIO_LIGHT,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> +				      BIT(IIO_CHAN_INFO_CALIBBIAS) |
> +				      BIT(IIO_CHAN_INFO_CALIBSCALE) |
> +				      BIT(IIO_CHAN_INFO_INT_TIME),
> +	},
> +};
> +
> +static int tsl2583_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> +
> +	mutex_lock(&chip->als_mutex);
> +
> +	if (chip->suspended) {
> +		ret = -EBUSY;
> +		goto read_done;
> +	}
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->type == IIO_LIGHT) {
> +			ret = tsl2583_get_lux(indio_dev);
> +			if (ret < 0)
> +				goto read_done;
> +
> +			/*
> +			 * From page 20 of the TSL2581, TSL2583 data
> +			 * sheet (TAOS134 − MARCH 2011):
> +			 *
> +			 * One of the photodiodes (channel 0) is
> +			 * sensitive to both visible and infrared light,
> +			 * while the second photodiode (channel 1) is
> +			 * sensitive primarily to infrared light.
> +			 */
> +			if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
> +				*val = chip->als_cur_info.als_ch0;
> +			else
> +				*val = chip->als_cur_info.als_ch1;
> +
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_PROCESSED:
> +		if (chan->type == IIO_LIGHT) {
> +			ret = tsl2583_get_lux(indio_dev);
> +			if (ret < 0)
> +				goto read_done;
> +
> +			*val = ret;
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = chip->als_settings.als_gain_trim;
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = gainadj[chip->als_settings.als_gain].mean;
> +			ret = IIO_VAL_INT;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT) {
> +			*val = 0;
> +			*val2 = chip->als_settings.als_time;
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +read_done:
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return ret;
> +}
> +
> +static int tsl2583_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> +
> +	mutex_lock(&chip->als_mutex);
> +
> +	if (chip->suspended) {
> +		ret = -EBUSY;
> +		goto write_done;
> +	}
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_CALIBBIAS:
> +		if (chan->type == IIO_LIGHT) {
> +			chip->als_settings.als_gain_trim = val;
> +			ret = 0;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_CALIBSCALE:
> +		if (chan->type == IIO_LIGHT) {
> +			unsigned int i;
> +
> +			for (i = 0; i < ARRAY_SIZE(gainadj); i++) {
> +				if (gainadj[i].mean == val) {
> +					chip->als_settings.als_gain = i;
> +					ret = tsl2583_set_als_gain(chip);
> +					break;
> +				}
> +			}
> +		}
> +		break;
> +	case IIO_CHAN_INFO_INT_TIME:
> +		if (chan->type == IIO_LIGHT && !val && val2 >= 50 &&
> +		    val2 <= 650 && !(val2 % 50)) {
> +			chip->als_settings.als_time = val2;
> +			ret = tsl2583_set_als_time(chip);
> +		}
> +		break;
> +	default:
> +		break;
> +	}
> +
> +write_done:
> +	mutex_unlock(&chip->als_mutex);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info tsl2583_info = {
> +	.attrs = &tsl2583_attribute_group,
> +	.driver_module = THIS_MODULE,
> +	.read_raw = tsl2583_read_raw,
> +	.write_raw = tsl2583_write_raw,
> +};
> +
> +/*
> + * Client probe function - When a valid device is found, the driver's device
> + * data structure is updated, and initialization completes successfully.
> + */
Can't honestly say this comment adds anything. Maybe I'm being fussy ;)
> +static int tsl2583_probe(struct i2c_client *clientp,
> +			 const struct i2c_device_id *idp)
> +{
> +	int ret;
> +	struct tsl2583_chip *chip;
> +	struct iio_dev *indio_dev;
> +
> +	if (!i2c_check_functionality(clientp->adapter,
> +				     I2C_FUNC_SMBUS_BYTE_DATA)) {
> +		dev_err(&clientp->dev, "%s: i2c smbus byte data functionality is unsupported\n",
> +			__func__);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&clientp->dev, sizeof(*chip));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	chip = iio_priv(indio_dev);
> +	chip->client = clientp;
> +	i2c_set_clientdata(clientp, indio_dev);
> +
> +	mutex_init(&chip->als_mutex);
> +	chip->suspended = true;
> +
> +	ret = i2c_smbus_read_byte_data(clientp,
> +				       TSL2583_CMD_REG | TSL2583_CHIPID);
> +	if (ret < 0) {
> +		dev_err(&clientp->dev,
> +			"%s failed to read the chip ID register\n", __func__);
> +		return ret;
> +	}
> +
> +	if ((ret & TSL2583_CHIP_ID_MASK) != TSL2583_CHIP_ID) {
> +		dev_err(&clientp->dev, "%s received an unknown chip ID %x\n",
> +			__func__, ret);
> +		return -EINVAL;
> +	}
> +
> +	indio_dev->info = &tsl2583_info;
> +	indio_dev->channels = tsl2583_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tsl2583_channels);
> +	indio_dev->dev.parent = &clientp->dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->name = chip->client->name;
> +
> +	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> +	if (ret) {
> +		dev_err(&clientp->dev, "%s: iio registration failed\n",
> +			__func__);
> +		return ret;
> +	}
> +
> +	/* Load up the V2 defaults (these are hard coded defaults for now) */
> +	tsl2583_defaults(chip);
> +
> +	/* Make sure the chip is on */
> +	ret = tsl2583_chip_init_and_power_on(indio_dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	dev_info(&clientp->dev, "Light sensor found.\n");
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int tsl2583_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret = 0;
Why is this being initialized?
> +
> +	mutex_lock(&chip->als_mutex);
> +
> +	ret = tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_OFF);
> +	chip->suspended = true;
> +
> +	mutex_unlock(&chip->als_mutex);
blank line here would be nice.
> +	return ret;
> +}
> +
> +static int tsl2583_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct tsl2583_chip *chip = iio_priv(indio_dev);
> +	int ret = 0;
or this one...
> +
> +	mutex_lock(&chip->als_mutex);
> +
> +	ret = tsl2583_chip_init_and_power_on(indio_dev);
> +
> +	mutex_unlock(&chip->als_mutex);
and here.
> +	return ret;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(tsl2583_pm_ops, tsl2583_suspend, tsl2583_resume);
> +#define TAOS_PM_OPS (&tsl2583_pm_ops)
> +#else
> +#define TAOS_PM_OPS NULL
> +#endif
> +
> +static struct i2c_device_id tsl2583_idtable[] = {
> +	{ "tsl2580", 0 },
> +	{ "tsl2581", 1 },
> +	{ "tsl2583", 2 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, tsl2583_idtable);
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id taos2583_of_match[] = {
> +	{ .compatible = "amstaos,tsl2580", },
> +	{ .compatible = "amstaos,tsl2581", },
> +	{ .compatible = "amstaos,tsl2583", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, taos2583_of_match);
> +#else
> +#define taos2583_of_match NULL
> +#endif
> +
> +/* Driver definition */
> +static struct i2c_driver tsl2583_driver = {
> +	.driver = {
> +		.name = "tsl2583",
> +		.pm = TAOS_PM_OPS,
> +		.of_match_table = taos2583_of_match,
> +	},
> +	.id_table = tsl2583_idtable,
> +	.probe = tsl2583_probe,
> +};
> +module_i2c_driver(tsl2583_driver);
> +
> +MODULE_AUTHOR("J. August Brenner <jbrenner@taosinc.com> and Brian Masney <masneyb@onstation.org>");
> +MODULE_DESCRIPTION("TAOS tsl2583 ambient light sensor driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/staging/iio/Documentation/light/sysfs-bus-iio-light-tsl2583 b/drivers/staging/iio/Documentation/light/sysfs-bus-iio-light-tsl2583
> deleted file mode 100644
> index a2e1996..0000000
> --- a/drivers/staging/iio/Documentation/light/sysfs-bus-iio-light-tsl2583
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -What:		/sys/bus/iio/devices/device[n]/in_illuminance_calibrate
> -KernelVersion:	2.6.37
> -Contact:	linux-iio@vger.kernel.org
> -Description:
> -		This property causes an internal calibration of the als gain trim
> -		value which is later used in calculating illuminance in lux.
> -
> -What:		/sys/bus/iio/devices/device[n]/in_illuminance_lux_table
> -KernelVersion:	2.6.37
> -Contact:	linux-iio@vger.kernel.org
> -Description:
> -		This property gets/sets the table of coefficients
> -		used in calculating illuminance in lux.
> -
> -What:		/sys/bus/iio/devices/device[n]/in_illuminance_input_target
> -KernelVersion:	2.6.37
> -Contact:	linux-iio@vger.kernel.org
> -Description:
> -		This property is the known externally illuminance (in lux).
> -		It is used in the process of calibrating the device accuracy.
> diff --git a/drivers/staging/iio/light/Kconfig b/drivers/staging/iio/light/Kconfig
> index dbf22d3..4fbf629 100644
> --- a/drivers/staging/iio/light/Kconfig
> +++ b/drivers/staging/iio/light/Kconfig
> @@ -13,13 +13,6 @@ config SENSORS_ISL29028
>  	 Proximity value via iio. The ISL29028 provides the concurrent sensing
>  	 of ambient light and proximity.
>  
> -config TSL2583
> -	tristate "TAOS TSL2580, TSL2581 and TSL2583 light-to-digital converters"
> -	depends on I2C
> -	help
> -	 Provides support for the TAOS tsl2580, tsl2581 and tsl2583 devices.
> -	 Access ALS data via iio, sysfs.
> -
>  config TSL2x7x
>  	tristate "TAOS TSL/TMD2x71 and TSL/TMD2x72 Family of light and proximity sensors"
>  	depends on I2C
> diff --git a/drivers/staging/iio/light/Makefile b/drivers/staging/iio/light/Makefile
> index 6480856..f8693e9 100644
> --- a/drivers/staging/iio/light/Makefile
> +++ b/drivers/staging/iio/light/Makefile
> @@ -3,5 +3,4 @@
>  #
>  
>  obj-$(CONFIG_SENSORS_ISL29028)	+= isl29028.o
> -obj-$(CONFIG_TSL2583)	+= tsl2583.o
>  obj-$(CONFIG_TSL2x7x)	+= tsl2x7x_core.o
> diff --git a/drivers/staging/iio/light/tsl2583.c b/drivers/staging/iio/light/tsl2583.c
> deleted file mode 100644
> index 5cb4ae3..0000000
> --- a/drivers/staging/iio/light/tsl2583.c
> +++ /dev/null
> @@ -1,916 +0,0 @@
> -/*
> - * Device driver for monitoring ambient light intensity (lux)
> - * within the TAOS tsl258x family of devices (tsl2580, tsl2581).
> - *
> - * Copyright (c) 2011, TAOS Corporation.
> - * Copyright (c) 2016 Brian Masney <masneyb@onstation.org>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful, but WITHOUT
> - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> - * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> - * more details.
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/i2c.h>
> -#include <linux/errno.h>
> -#include <linux/delay.h>
> -#include <linux/string.h>
> -#include <linux/mutex.h>
> -#include <linux/unistd.h>
> -#include <linux/slab.h>
> -#include <linux/module.h>
> -#include <linux/iio/iio.h>
> -#include <linux/iio/sysfs.h>
> -
> -/* Device Registers and Masks */
> -#define TSL2583_CNTRL			0x00
> -#define TSL2583_ALS_TIME		0X01
> -#define TSL2583_INTERRUPT		0x02
> -#define TSL2583_GAIN			0x07
> -#define TSL2583_REVID			0x11
> -#define TSL2583_CHIPID			0x12
> -#define TSL2583_ALS_CHAN0LO		0x14
> -#define TSL2583_ALS_CHAN0HI		0x15
> -#define TSL2583_ALS_CHAN1LO		0x16
> -#define TSL2583_ALS_CHAN1HI		0x17
> -#define TSL2583_TMR_LO			0x18
> -#define TSL2583_TMR_HI			0x19
> -
> -/* tsl2583 cmd reg masks */
> -#define TSL2583_CMD_REG			0x80
> -#define TSL2583_CMD_SPL_FN		0x60
> -#define TSL2583_CMD_ALS_INT_CLR		0x01
> -
> -/* tsl2583 cntrl reg masks */
> -#define TSL2583_CNTL_ADC_ENBL		0x02
> -#define TSL2583_CNTL_PWR_OFF		0x00
> -#define TSL2583_CNTL_PWR_ON		0x01
> -
> -/* tsl2583 status reg masks */
> -#define TSL2583_STA_ADC_VALID		0x01
> -#define TSL2583_STA_ADC_INTR		0x10
> -
> -/* Lux calculation constants */
> -#define	TSL2583_LUX_CALC_OVER_FLOW	65535
> -
> -#define TSL2583_INTERRUPT_DISABLED	0x00
> -
> -#define TSL2583_CHIP_ID			0x90
> -#define TSL2583_CHIP_ID_MASK		0xf0
> -
> -/* Per-device data */
> -struct tsl2583_als_info {
> -	u16 als_ch0;
> -	u16 als_ch1;
> -	u16 lux;
> -};
> -
> -struct tsl2583_lux {
> -	unsigned int ratio;
> -	unsigned int ch0;
> -	unsigned int ch1;
> -};
> -
> -static const struct tsl2583_lux tsl2583_default_lux[] = {
> -	{  9830,  8520, 15729 },
> -	{ 12452, 10807, 23344 },
> -	{ 14746,  6383, 11705 },
> -	{ 17695,  4063,  6554 },
> -	{     0,     0,     0 }  /* Termination segment */
> -};
> -
> -struct tsl2583_settings {
> -	int als_time;
> -	int als_gain;
> -	int als_gain_trim;
> -	int als_cal_target;
> -
> -	/*
> -	 * This structure is intentionally large to accommodate updates via
> -	 * sysfs. Sized to 11 = max 10 segments + 1 termination segment.
> -	 * Assumption is that one and only one type of glass used.
> -	 */
> -	struct tsl2583_lux als_device_lux[11];
> -};
> -
> -struct tsl2583_chip {
> -	struct mutex als_mutex;
> -	struct i2c_client *client;
> -	struct tsl2583_als_info als_cur_info;
> -	struct tsl2583_settings als_settings;
> -	int als_time_scale;
> -	int als_saturation;
> -	bool suspended;
> -};
> -
> -struct gainadj {
> -	s16 ch0;
> -	s16 ch1;
> -	s16 mean;
> -};
> -
> -/* Index = (0 - 3) Used to validate the gain selection index */
> -static const struct gainadj gainadj[] = {
> -	{ 1, 1, 1 },
> -	{ 8, 8, 8 },
> -	{ 16, 16, 16 },
> -	{ 107, 115, 111 }
> -};
> -
> -/*
> - * Provides initial operational parameter defaults.
> - * These defaults may be changed through the device's sysfs files.
> - */
> -static void tsl2583_defaults(struct tsl2583_chip *chip)
> -{
> -	/*
> -	 * The integration time must be a multiple of 50ms and within the
> -	 * range [50, 600] ms.
> -	 */
> -	chip->als_settings.als_time = 100;
> -
> -	/*
> -	 * This is an index into the gainadj table. Assume clear glass as the
> -	 * default.
> -	 */
> -	chip->als_settings.als_gain = 0;
> -
> -	/* Default gain trim to account for aperture effects */
> -	chip->als_settings.als_gain_trim = 1000;
> -
> -	/* Known external ALS reading used for calibration */
> -	chip->als_settings.als_cal_target = 130;
> -
> -	/* Default lux table. */
> -	memcpy(chip->als_settings.als_device_lux, tsl2583_default_lux,
> -	       sizeof(tsl2583_default_lux));
> -}
> -
> -/*
> - * Reads and calculates current lux value.
> - * The raw ch0 and ch1 values of the ambient light sensed in the last
> - * integration cycle are read from the device.
> - * Time scale factor array values are adjusted based on the integration time.
> - * The raw values are multiplied by a scale factor, and device gain is obtained
> - * using gain index. Limit checks are done next, then the ratio of a multiple
> - * of ch1 value, to the ch0 value, is calculated. The array als_device_lux[]
> - * declared above is then scanned to find the first ratio value that is just
> - * above the ratio we just calculated. The ch0 and ch1 multiplier constants in
> - * the array are then used along with the time scale factor array values, to
> - * calculate the lux.
> - */
> -static int tsl2583_get_lux(struct iio_dev *indio_dev)
> -{
> -	u16 ch0, ch1; /* separated ch0/ch1 data from device */
> -	u32 lux; /* raw lux calculated from device data */
> -	u64 lux64;
> -	u32 ratio;
> -	u8 buf[5];
> -	struct tsl2583_lux *p;
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int i, ret;
> -
> -	ret = i2c_smbus_read_byte_data(chip->client, TSL2583_CMD_REG);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev, "%s failed to read CMD_REG register\n",
> -			__func__);
> -		goto done;
> -	}
> -
> -	/* is data new & valid */
> -	if (!(ret & TSL2583_STA_ADC_INTR)) {
> -		dev_err(&chip->client->dev, "%s: data not valid; returning last value\n",
> -			__func__);
> -		ret = chip->als_cur_info.lux; /* return LAST VALUE */
> -		goto done;
> -	}
> -
> -	for (i = 0; i < 4; i++) {
> -		int reg = TSL2583_CMD_REG | (TSL2583_ALS_CHAN0LO + i);
> -
> -		ret = i2c_smbus_read_byte_data(chip->client, reg);
> -		if (ret < 0) {
> -			dev_err(&chip->client->dev, "%s failed to read register %x\n",
> -				__func__, reg);
> -			goto done;
> -		}
> -		buf[i] = ret;
> -	}
> -
> -	/*
> -	 * clear status, really interrupt status (interrupts are off), but
> -	 * we use the bit anyway - don't forget 0x80 - this is a command
> -	 */
> -	ret = i2c_smbus_write_byte(chip->client,
> -				   (TSL2583_CMD_REG | TSL2583_CMD_SPL_FN |
> -				    TSL2583_CMD_ALS_INT_CLR));
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev, "%s failed to clear the interrupt bit\n",
> -			__func__);
> -		goto done; /* have no data, so return failure */
> -	}
> -
> -	/* extract ALS/lux data */
> -	ch0 = le16_to_cpup((const __le16 *)&buf[0]);
> -	ch1 = le16_to_cpup((const __le16 *)&buf[2]);
> -
> -	chip->als_cur_info.als_ch0 = ch0;
> -	chip->als_cur_info.als_ch1 = ch1;
> -
> -	if ((ch0 >= chip->als_saturation) || (ch1 >= chip->als_saturation))
> -		goto return_max;
> -
> -	if (!ch0) {
> -		/* have no data, so return 0 */
> -		ret = 0;
> -		chip->als_cur_info.lux = 0;
> -		goto done;
> -	}
> -
> -	/* calculate ratio */
> -	ratio = (ch1 << 15) / ch0;
> -
> -	/* convert to unscaled lux using the pointer to the table */
> -	for (p = (struct tsl2583_lux *)chip->als_settings.als_device_lux;
> -	     p->ratio != 0 && p->ratio < ratio; p++)
> -		;
> -
> -	if (p->ratio == 0) {
> -		lux = 0;
> -	} else {
> -		u32 ch0lux, ch1lux;
> -
> -		ch0lux = ((ch0 * p->ch0) +
> -			  (gainadj[chip->als_settings.als_gain].ch0 >> 1))
> -			 / gainadj[chip->als_settings.als_gain].ch0;
> -		ch1lux = ((ch1 * p->ch1) +
> -			  (gainadj[chip->als_settings.als_gain].ch1 >> 1))
> -			 / gainadj[chip->als_settings.als_gain].ch1;
> -
> -		/* note: lux is 31 bit max at this point */
> -		if (ch1lux > ch0lux) {
> -			dev_dbg(&chip->client->dev, "%s: No Data - Returning 0\n",
> -				__func__);
> -			ret = 0;
> -			chip->als_cur_info.lux = 0;
> -			goto done;
> -		}
> -
> -		lux = ch0lux - ch1lux;
> -	}
> -
> -	/* adjust for active time scale */
> -	if (chip->als_time_scale == 0)
> -		lux = 0;
> -	else
> -		lux = (lux + (chip->als_time_scale >> 1)) /
> -			chip->als_time_scale;
> -
> -	/*
> -	 * Adjust for active gain scale.
> -	 * The tsl2583_default_lux tables above have a factor of 8192 built in,
> -	 * so we need to shift right.
> -	 * User-specified gain provides a multiplier.
> -	 * Apply user-specified gain before shifting right to retain precision.
> -	 * Use 64 bits to avoid overflow on multiplication.
> -	 * Then go back to 32 bits before division to avoid using div_u64().
> -	 */
> -	lux64 = lux;
> -	lux64 = lux64 * chip->als_settings.als_gain_trim;
> -	lux64 >>= 13;
> -	lux = lux64;
> -	lux = (lux + 500) / 1000;
> -
> -	if (lux > TSL2583_LUX_CALC_OVER_FLOW) { /* check for overflow */
> -return_max:
> -		lux = TSL2583_LUX_CALC_OVER_FLOW;
> -	}
> -
> -	/* Update the structure with the latest VALID lux. */
> -	chip->als_cur_info.lux = lux;
> -	ret = lux;
> -
> -done:
> -	return ret;
> -}
> -
> -/*
> - * Obtain single reading and calculate the als_gain_trim (later used
> - * to derive actual lux).
> - * Return updated gain_trim value.
> - */
> -static int tsl2583_als_calibrate(struct iio_dev *indio_dev)
> -{
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	unsigned int gain_trim_val;
> -	int ret;
> -	int lux_val;
> -
> -	ret = i2c_smbus_read_byte_data(chip->client,
> -				       TSL2583_CMD_REG | TSL2583_CNTRL);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev,
> -			"%s failed to read from the CNTRL register\n",
> -			__func__);
> -		return ret;
> -	}
> -
> -	if ((ret & (TSL2583_CNTL_ADC_ENBL | TSL2583_CNTL_PWR_ON))
> -			!= (TSL2583_CNTL_ADC_ENBL | TSL2583_CNTL_PWR_ON)) {
> -		dev_err(&chip->client->dev,
> -			"%s failed: device not powered on with ADC enabled\n",
> -			__func__);
> -		return -EINVAL;
> -	} else if ((ret & TSL2583_STA_ADC_VALID) != TSL2583_STA_ADC_VALID) {
> -		dev_err(&chip->client->dev,
> -			"%s failed: STATUS - ADC not valid.\n", __func__);
> -		return -ENODATA;
> -	}
> -
> -	lux_val = tsl2583_get_lux(indio_dev);
> -	if (lux_val < 0) {
> -		dev_err(&chip->client->dev, "%s failed to get lux\n", __func__);
> -		return lux_val;
> -	}
> -
> -	gain_trim_val = (unsigned int)(((chip->als_settings.als_cal_target)
> -			* chip->als_settings.als_gain_trim) / lux_val);
> -	if ((gain_trim_val < 250) || (gain_trim_val > 4000)) {
> -		dev_err(&chip->client->dev,
> -			"%s failed: trim_val of %d is not within the range [250, 4000]\n",
> -			__func__, gain_trim_val);
> -		return -ENODATA;
> -	}
> -
> -	chip->als_settings.als_gain_trim = (int)gain_trim_val;
> -
> -	return 0;
> -}
> -
> -static int tsl2583_set_als_time(struct tsl2583_chip *chip)
> -{
> -	int als_count, als_time, ret;
> -	u8 val;
> -
> -	/* determine als integration register */
> -	als_count = (chip->als_settings.als_time * 100 + 135) / 270;
> -	if (!als_count)
> -		als_count = 1; /* ensure at least one cycle */
> -
> -	/* convert back to time (encompasses overrides) */
> -	als_time = (als_count * 27 + 5) / 10;
> -
> -	val = 256 - als_count;
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2583_CMD_REG | TSL2583_ALS_TIME,
> -					val);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev, "%s failed to set the als time to %d\n",
> -			__func__, val);
> -		return ret;
> -	}
> -
> -	/* set chip struct re scaling and saturation */
> -	chip->als_saturation = als_count * 922; /* 90% of full scale */
> -	chip->als_time_scale = (als_time + 25) / 50;
> -
> -	return ret;
> -}
> -
> -static int tsl2583_set_als_gain(struct tsl2583_chip *chip)
> -{
> -	int ret;
> -
> -	/* Set the gain based on als_settings struct */
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2583_CMD_REG | TSL2583_GAIN,
> -					chip->als_settings.als_gain);
> -	if (ret < 0)
> -		dev_err(&chip->client->dev, "%s failed to set the gain to %d\n",
> -			__func__, chip->als_settings.als_gain);
> -
> -	return ret;
> -}
> -
> -static int tsl2583_set_power_state(struct tsl2583_chip *chip, u8 state)
> -{
> -	int ret;
> -
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2583_CMD_REG | TSL2583_CNTRL, state);
> -	if (ret < 0)
> -		dev_err(&chip->client->dev, "%s failed to set the power state to %d\n",
> -			__func__, state);
> -
> -	return ret;
> -}
> -
> -/*
> - * Turn the device on.
> - * Configuration must be set before calling this function.
> - */
> -static int tsl2583_chip_init_and_power_on(struct iio_dev *indio_dev)
> -{
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int ret;
> -
> -	/* Power on the device; ADC off. */
> -	ret = tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_ON);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = i2c_smbus_write_byte_data(chip->client,
> -					TSL2583_CMD_REG | TSL2583_INTERRUPT,
> -					TSL2583_INTERRUPT_DISABLED);
> -	if (ret < 0) {
> -		dev_err(&chip->client->dev, "%s failed to disable interrupts\n",
> -			__func__);
> -		return ret;
> -	}
> -
> -	ret = tsl2583_set_als_time(chip);
> -	if (ret < 0)
> -		return ret;
> -
> -	ret = tsl2583_set_als_gain(chip);
> -	if (ret < 0)
> -		return ret;
> -
> -	usleep_range(3000, 3500);
> -
> -	ret = tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_ON |
> -					    TSL2583_CNTL_ADC_ENBL);
> -	if (ret < 0)
> -		return ret;
> -
> -	chip->suspended = false;
> -
> -	return ret;
> -}
> -
> -/* Sysfs Interface Functions */
> -
> -static ssize_t in_illuminance_input_target_show(struct device *dev,
> -						struct device_attribute *attr,
> -						char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int ret;
> -
> -	mutex_lock(&chip->als_mutex);
> -	ret = sprintf(buf, "%d\n", chip->als_settings.als_cal_target);
> -	mutex_unlock(&chip->als_mutex);
> -
> -	return ret;
> -}
> -
> -static ssize_t in_illuminance_input_target_store(struct device *dev,
> -						 struct device_attribute *attr,
> -						 const char *buf, size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int value;
> -
> -	if (kstrtoint(buf, 0, &value) || !value)
> -		return -EINVAL;
> -
> -	mutex_lock(&chip->als_mutex);
> -	chip->als_settings.als_cal_target = value;
> -	mutex_unlock(&chip->als_mutex);
> -
> -	return len;
> -}
> -
> -static ssize_t in_illuminance_calibrate_store(struct device *dev,
> -					      struct device_attribute *attr,
> -					      const char *buf, size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int value, ret;
> -
> -	if (kstrtoint(buf, 0, &value) || value != 1)
> -		return -EINVAL;
> -
> -	mutex_lock(&chip->als_mutex);
> -
> -	if (chip->suspended) {
> -		ret = -EBUSY;
> -		goto done;
> -	}
> -
> -	ret = tsl2583_als_calibrate(indio_dev);
> -	if (ret < 0)
> -		goto done;
> -
> -	ret = len;
> -done:
> -	mutex_unlock(&chip->als_mutex);
> -
> -	return ret;
> -}
> -
> -static ssize_t in_illuminance_lux_table_show(struct device *dev,
> -					     struct device_attribute *attr,
> -					     char *buf)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	unsigned int i;
> -	int offset = 0;
> -
> -	for (i = 0; i < ARRAY_SIZE(chip->als_settings.als_device_lux); i++) {
> -		offset += sprintf(buf + offset, "%u,%u,%u,",
> -				  chip->als_settings.als_device_lux[i].ratio,
> -				  chip->als_settings.als_device_lux[i].ch0,
> -				  chip->als_settings.als_device_lux[i].ch1);
> -		if (chip->als_settings.als_device_lux[i].ratio == 0) {
> -			/*
> -			 * We just printed the first "0" entry.
> -			 * Now get rid of the extra "," and break.
> -			 */
> -			offset--;
> -			break;
> -		}
> -	}
> -
> -	offset += sprintf(buf + offset, "\n");
> -
> -	return offset;
> -}
> -
> -static ssize_t in_illuminance_lux_table_store(struct device *dev,
> -					      struct device_attribute *attr,
> -					      const char *buf, size_t len)
> -{
> -	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	const unsigned int max_ints =
> -		ARRAY_SIZE(chip->als_settings.als_device_lux) * 3;
> -	int value[max_ints];
> -	int ret = -EINVAL;
> -	unsigned int n;
> -
> -	mutex_lock(&chip->als_mutex);
> -
> -	get_options(buf, ARRAY_SIZE(value), value);
> -
> -	/*
> -	 * We now have an array of ints starting at value[1], and
> -	 * enumerated by value[0].
> -	 * We expect each group of three ints is one table entry,
> -	 * and the last table entry is all 0.
> -	 */
> -	n = value[0];
> -	if ((n % 3) || n < 6 || n > max_ints) {
> -		dev_err(dev,
> -			"%s: The number of entries in the lux table must be a multiple of 3 and within the range [6, %d]",
> -			__func__, max_ints);
> -		goto done;
> -	}
> -	if ((value[n - 2] | value[n - 1] | value[n]) != 0) {
> -		dev_err(dev, "%s: The last 3 entries in the lux table must be zeros.\n",
> -			__func__);
> -		goto done;
> -	}
> -
> -	/* Zero out the table */
> -	memset(chip->als_settings.als_device_lux, 0,
> -	       sizeof(chip->als_settings.als_device_lux));
> -	memcpy(chip->als_settings.als_device_lux, &value[1],
> -	       value[0] * sizeof(unsigned int));
> -
> -	ret = len;
> -
> -done:
> -	mutex_unlock(&chip->als_mutex);
> -
> -	return ret;
> -}
> -
> -static IIO_CONST_ATTR(in_illuminance_calibscale_available, "1 8 16 111");
> -static IIO_CONST_ATTR(in_illuminance_integration_time_available,
> -		      "0.000050 0.000100 0.000150 0.000200 0.000250 0.000300 0.000350 0.000400 0.000450 0.000500 0.000550 0.000600 0.000650");
> -static IIO_DEVICE_ATTR_RW(in_illuminance_input_target, 0);
> -static IIO_DEVICE_ATTR_WO(in_illuminance_calibrate, 0);
> -static IIO_DEVICE_ATTR_RW(in_illuminance_lux_table, 0);
> -
> -static struct attribute *sysfs_attrs_ctrl[] = {
> -	&iio_const_attr_in_illuminance_calibscale_available.dev_attr.attr,
> -	&iio_const_attr_in_illuminance_integration_time_available.dev_attr.attr,
> -	&iio_dev_attr_in_illuminance_input_target.dev_attr.attr,
> -	&iio_dev_attr_in_illuminance_calibrate.dev_attr.attr,
> -	&iio_dev_attr_in_illuminance_lux_table.dev_attr.attr,
> -	NULL
> -};
> -
> -static const struct attribute_group tsl2583_attribute_group = {
> -	.attrs = sysfs_attrs_ctrl,
> -};
> -
> -static const struct iio_chan_spec tsl2583_channels[] = {
> -	{
> -		.type = IIO_LIGHT,
> -		.modified = 1,
> -		.channel2 = IIO_MOD_LIGHT_IR,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -	},
> -	{
> -		.type = IIO_LIGHT,
> -		.modified = 1,
> -		.channel2 = IIO_MOD_LIGHT_BOTH,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> -	},
> -	{
> -		.type = IIO_LIGHT,
> -		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
> -				      BIT(IIO_CHAN_INFO_CALIBBIAS) |
> -				      BIT(IIO_CHAN_INFO_CALIBSCALE) |
> -				      BIT(IIO_CHAN_INFO_INT_TIME),
> -	},
> -};
> -
> -static int tsl2583_read_raw(struct iio_dev *indio_dev,
> -			    struct iio_chan_spec const *chan,
> -			    int *val, int *val2, long mask)
> -{
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int ret = -EINVAL;
> -
> -	mutex_lock(&chip->als_mutex);
> -
> -	if (chip->suspended) {
> -		ret = -EBUSY;
> -		goto read_done;
> -	}
> -
> -	switch (mask) {
> -	case IIO_CHAN_INFO_RAW:
> -		if (chan->type == IIO_LIGHT) {
> -			ret = tsl2583_get_lux(indio_dev);
> -			if (ret < 0)
> -				goto read_done;
> -
> -			/*
> -			 * From page 20 of the TSL2581, TSL2583 data
> -			 * sheet (TAOS134 − MARCH 2011):
> -			 *
> -			 * One of the photodiodes (channel 0) is
> -			 * sensitive to both visible and infrared light,
> -			 * while the second photodiode (channel 1) is
> -			 * sensitive primarily to infrared light.
> -			 */
> -			if (chan->channel2 == IIO_MOD_LIGHT_BOTH)
> -				*val = chip->als_cur_info.als_ch0;
> -			else
> -				*val = chip->als_cur_info.als_ch1;
> -
> -			ret = IIO_VAL_INT;
> -		}
> -		break;
> -	case IIO_CHAN_INFO_PROCESSED:
> -		if (chan->type == IIO_LIGHT) {
> -			ret = tsl2583_get_lux(indio_dev);
> -			if (ret < 0)
> -				goto read_done;
> -
> -			*val = ret;
> -			ret = IIO_VAL_INT;
> -		}
> -		break;
> -	case IIO_CHAN_INFO_CALIBBIAS:
> -		if (chan->type == IIO_LIGHT) {
> -			*val = chip->als_settings.als_gain_trim;
> -			ret = IIO_VAL_INT;
> -		}
> -		break;
> -	case IIO_CHAN_INFO_CALIBSCALE:
> -		if (chan->type == IIO_LIGHT) {
> -			*val = gainadj[chip->als_settings.als_gain].mean;
> -			ret = IIO_VAL_INT;
> -		}
> -		break;
> -	case IIO_CHAN_INFO_INT_TIME:
> -		if (chan->type == IIO_LIGHT) {
> -			*val = 0;
> -			*val2 = chip->als_settings.als_time;
> -			ret = IIO_VAL_INT_PLUS_MICRO;
> -		}
> -		break;
> -	default:
> -		break;
> -	}
> -
> -read_done:
> -	mutex_unlock(&chip->als_mutex);
> -
> -	return ret;
> -}
> -
> -static int tsl2583_write_raw(struct iio_dev *indio_dev,
> -			     struct iio_chan_spec const *chan,
> -			     int val, int val2, long mask)
> -{
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int ret = -EINVAL;
> -
> -	mutex_lock(&chip->als_mutex);
> -
> -	if (chip->suspended) {
> -		ret = -EBUSY;
> -		goto write_done;
> -	}
> -
> -	switch (mask) {
> -	case IIO_CHAN_INFO_CALIBBIAS:
> -		if (chan->type == IIO_LIGHT) {
> -			chip->als_settings.als_gain_trim = val;
> -			ret = 0;
> -		}
> -		break;
> -	case IIO_CHAN_INFO_CALIBSCALE:
> -		if (chan->type == IIO_LIGHT) {
> -			unsigned int i;
> -
> -			for (i = 0; i < ARRAY_SIZE(gainadj); i++) {
> -				if (gainadj[i].mean == val) {
> -					chip->als_settings.als_gain = i;
> -					ret = tsl2583_set_als_gain(chip);
> -					break;
> -				}
> -			}
> -		}
> -		break;
> -	case IIO_CHAN_INFO_INT_TIME:
> -		if (chan->type == IIO_LIGHT && !val && val2 >= 50 &&
> -		    val2 <= 650 && !(val2 % 50)) {
> -			chip->als_settings.als_time = val2;
> -			ret = tsl2583_set_als_time(chip);
> -		}
> -		break;
> -	default:
> -		break;
> -	}
> -
> -write_done:
> -	mutex_unlock(&chip->als_mutex);
> -
> -	return ret;
> -}
> -
> -static const struct iio_info tsl2583_info = {
> -	.attrs = &tsl2583_attribute_group,
> -	.driver_module = THIS_MODULE,
> -	.read_raw = tsl2583_read_raw,
> -	.write_raw = tsl2583_write_raw,
> -};
> -
> -/*
> - * Client probe function - When a valid device is found, the driver's device
> - * data structure is updated, and initialization completes successfully.
> - */
> -static int tsl2583_probe(struct i2c_client *clientp,
> -			 const struct i2c_device_id *idp)
> -{
> -	int ret;
> -	struct tsl2583_chip *chip;
> -	struct iio_dev *indio_dev;
> -
> -	if (!i2c_check_functionality(clientp->adapter,
> -				     I2C_FUNC_SMBUS_BYTE_DATA)) {
> -		dev_err(&clientp->dev, "%s: i2c smbus byte data functionality is unsupported\n",
> -			__func__);
> -		return -EOPNOTSUPP;
> -	}
> -
> -	indio_dev = devm_iio_device_alloc(&clientp->dev, sizeof(*chip));
> -	if (!indio_dev)
> -		return -ENOMEM;
> -
> -	chip = iio_priv(indio_dev);
> -	chip->client = clientp;
> -	i2c_set_clientdata(clientp, indio_dev);
> -
> -	mutex_init(&chip->als_mutex);
> -	chip->suspended = true;
> -
> -	ret = i2c_smbus_read_byte_data(clientp,
> -				       TSL2583_CMD_REG | TSL2583_CHIPID);
> -	if (ret < 0) {
> -		dev_err(&clientp->dev,
> -			"%s failed to read the chip ID register\n", __func__);
> -		return ret;
> -	}
> -
> -	if ((ret & TSL2583_CHIP_ID_MASK) != TSL2583_CHIP_ID) {
> -		dev_err(&clientp->dev, "%s received an unknown chip ID %x\n",
> -			__func__, ret);
> -		return -EINVAL;
> -	}
> -
> -	indio_dev->info = &tsl2583_info;
> -	indio_dev->channels = tsl2583_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(tsl2583_channels);
> -	indio_dev->dev.parent = &clientp->dev;
> -	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->name = chip->client->name;
> -
> -	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> -	if (ret) {
> -		dev_err(&clientp->dev, "%s: iio registration failed\n",
> -			__func__);
> -		return ret;
> -	}
> -
> -	/* Load up the V2 defaults (these are hard coded defaults for now) */
> -	tsl2583_defaults(chip);
> -
> -	/* Make sure the chip is on */
> -	ret = tsl2583_chip_init_and_power_on(indio_dev);
> -	if (ret < 0)
> -		return ret;
> -
> -	dev_info(&clientp->dev, "Light sensor found.\n");
> -
> -	return 0;
> -}
> -
> -#ifdef CONFIG_PM_SLEEP
> -static int tsl2583_suspend(struct device *dev)
> -{
> -	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int ret = 0;
> -
> -	mutex_lock(&chip->als_mutex);
> -
> -	ret = tsl2583_set_power_state(chip, TSL2583_CNTL_PWR_OFF);
> -	chip->suspended = true;
> -
> -	mutex_unlock(&chip->als_mutex);
> -	return ret;
> -}
> -
> -static int tsl2583_resume(struct device *dev)
> -{
> -	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> -	struct tsl2583_chip *chip = iio_priv(indio_dev);
> -	int ret = 0;
> -
> -	mutex_lock(&chip->als_mutex);
> -
> -	ret = tsl2583_chip_init_and_power_on(indio_dev);
> -
> -	mutex_unlock(&chip->als_mutex);
> -	return ret;
> -}
> -
> -static SIMPLE_DEV_PM_OPS(tsl2583_pm_ops, tsl2583_suspend, tsl2583_resume);
> -#define TAOS_PM_OPS (&tsl2583_pm_ops)
> -#else
> -#define TAOS_PM_OPS NULL
> -#endif
> -
> -static struct i2c_device_id tsl2583_idtable[] = {
> -	{ "tsl2580", 0 },
> -	{ "tsl2581", 1 },
> -	{ "tsl2583", 2 },
> -	{}
> -};
> -MODULE_DEVICE_TABLE(i2c, tsl2583_idtable);
> -
> -#ifdef CONFIG_OF
> -static const struct of_device_id taos2583_of_match[] = {
> -	{ .compatible = "amstaos,tsl2580", },
> -	{ .compatible = "amstaos,tsl2581", },
> -	{ .compatible = "amstaos,tsl2583", },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, taos2583_of_match);
> -#else
> -#define taos2583_of_match NULL
> -#endif
> -
> -/* Driver definition */
> -static struct i2c_driver tsl2583_driver = {
> -	.driver = {
> -		.name = "tsl2583",
> -		.pm = TAOS_PM_OPS,
> -		.of_match_table = taos2583_of_match,
> -	},
> -	.id_table = tsl2583_idtable,
> -	.probe = tsl2583_probe,
> -};
> -module_i2c_driver(tsl2583_driver);
> -
> -MODULE_AUTHOR("J. August Brenner <jbrenner@taosinc.com> and Brian Masney <masneyb@onstation.org>");
> -MODULE_DESCRIPTION("TAOS tsl2583 ambient light sensor driver");
> -MODULE_LICENSE("GPL");
> 


^ permalink raw reply

* RE: [Resend] [PATCH] pci-hyperv: use kmalloc to allocate hypercall params buffer
From: Long Li @ 2016-11-08 20:49 UTC (permalink / raw)
  To: Long Li, Greg KH
  Cc: linux-pci@vger.kernel.org, Haiyang Zhang,
	linux-kernel@vger.kernel.org, Bjorn Helgaas,
	devel@linuxdriverproject.org
In-Reply-To: <BN3PR03MB2227A432B0288764BA9EE562CEA60@BN3PR03MB2227.namprd03.prod.outlook.com>



> -----Original Message-----
> From: devel [mailto:driverdev-devel-bounces@linuxdriverproject.org] On
> Behalf Of Long Li
> Sent: Tuesday, November 8, 2016 8:57 AM
> To: Greg KH <gregkh@linuxfoundation.org>
> Cc: linux-pci@vger.kernel.org; Haiyang Zhang <haiyangz@microsoft.com>;
> linux-kernel@vger.kernel.org; Bjorn Helgaas <bhelgaas@google.com>;
> devel@linuxdriverproject.org
> Subject: RE: [Resend] [PATCH] pci-hyperv: use kmalloc to allocate hypercall
> params buffer
> 
> This sender failed our fraud detection checks and may not be who they
> appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Monday, November 7, 2016 11:00 PM
> > To: Long Li <longli@microsoft.com>
> > Cc: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> > <haiyangz@microsoft.com>; Bjorn Helgaas <bhelgaas@google.com>;
> > devel@linuxdriverproject.org; linux-kernel@vger.kernel.org; linux-
> > pci@vger.kernel.org
> > Subject: Re: [Resend] [PATCH] pci-hyperv: use kmalloc to allocate
> > hypercall params buffer
> >
> > On Tue, Nov 08, 2016 at 12:14:14AM -0800, Long Li wrote:
> > > From: Long Li <longli@microsoft.com>
> > >
> > > hv_do_hypercall assumes that we pass a segment from a physically
> > continuous buffer. Buffer allocated on the stack may not work if
> > CONFIG_VMAP_STACK=y is set. Use kmalloc to allocate this buffer.
> >
> > Please wrap your changelog at 72 columns.
> >
> > >
> > > Signed-off-by: Long Li <longli@microsoft.com>
> > > Reported-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > ---
> > >  drivers/pci/host/pci-hyperv.c | 24 +++++++++++++-----------
> > >  1 file changed, 13 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/pci/host/pci-hyperv.c
> > > b/drivers/pci/host/pci-hyperv.c index 763ff87..97e6daf 100644
> > > --- a/drivers/pci/host/pci-hyperv.c
> > > +++ b/drivers/pci/host/pci-hyperv.c
> > > @@ -378,6 +378,7 @@ struct hv_pcibus_device {
> > >     struct msi_domain_info msi_info;
> > >     struct msi_controller msi_chip;
> > >     struct irq_domain *irq_domain;
> > > +   struct retarget_msi_interrupt retarget_msi_interrupt_params;
> >
> > Can you handle potentially unaligned accesses like this?  Is there
> > some lock preventing you from using this structure more than once at the
> same time?
> >
> > >  };
> > >
> > >  /*
> > > @@ -774,7 +775,7 @@ void hv_irq_unmask(struct irq_data *data)  {
> > >     struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
> > >     struct irq_cfg *cfg = irqd_cfg(data);
> > > -   struct retarget_msi_interrupt params;
> > > +   struct retarget_msi_interrupt *params;
> > >     struct hv_pcibus_device *hbus;
> > >     struct cpumask *dest;
> > >     struct pci_bus *pbus;
> > > @@ -785,23 +786,24 @@ void hv_irq_unmask(struct irq_data *data)
> > >     pdev = msi_desc_to_pci_dev(msi_desc);
> > >     pbus = pdev->bus;
> > >     hbus = container_of(pbus->sysdata, struct hv_pcibus_device,
> > > sysdata);
> > > -
> > > -   memset(&params, 0, sizeof(params));
> > > -   params.partition_id = HV_PARTITION_ID_SELF;
> > > -   params.source = 1; /* MSI(-X) */
> > > -   params.address = msi_desc->msg.address_lo;
> > > -   params.data = msi_desc->msg.data;
> > > -   params.device_id = (hbus->hdev->dev_instance.b[5] << 24) |
> > > +   params = &hbus->retarget_msi_interrupt_params;
> > > +
> > > +   memset(params, 0, sizeof(*params));
> > > +   params->partition_id = HV_PARTITION_ID_SELF;
> > > +   params->source = 1; /* MSI(-X) */
> > > +   params->address = msi_desc->msg.address_lo;
> > > +   params->data = msi_desc->msg.data;
> > > +   params->device_id = (hbus->hdev->dev_instance.b[5] << 24) |
> > >                        (hbus->hdev->dev_instance.b[4] << 16) |
> > >                        (hbus->hdev->dev_instance.b[7] << 8) |
> > >                        (hbus->hdev->dev_instance.b[6] & 0xf8) |
> > >                        PCI_FUNC(pdev->devfn);
> > > -   params.vector = cfg->vector;
> > > +   params->vector = cfg->vector;
> > >
> > >     for_each_cpu_and(cpu, dest, cpu_online_mask)
> > > -           params.vp_mask |= (1ULL <<
> > vmbus_cpu_number_to_vp_number(cpu));
> > > +           params->vp_mask |= (1ULL <<
> > vmbus_cpu_number_to_vp_number(cpu));
> > >
> > > -   hv_do_hypercall(HVCALL_RETARGET_INTERRUPT, &params, NULL);
> > > +   hv_do_hypercall(HVCALL_RETARGET_INTERRUPT, params, NULL);
> >
> > As you only use this in one spot, why not just allocate it here and
> > then free it?  Why add it to the pcibus device structure?
> 
> Thanks Greg. I will send a V2.

Sorry forgot to address the reason why we don't just allocate the buffer here. This function cannot fail. So it's better to pre-allocate the buffer.

> 
> >
> > thanks,
> >
> > greg k-h
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdriverd
> ev.linuxdriverproject.org%2Fmailman%2Flistinfo%2Fdriverdev-
> devel&data=02%7C01%7Clongli%40microsoft.com%7C6e28f5459da345cdb5e
> 408d407f836f0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636142
> 210041216500&sdata=vvVxGQUet7KMuRgs9%2BRbR8JE70rKF1AJo%2Fu3zx%
> 2FQNwY%3D&reserved=0

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.