Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Tearing down DMA transfer setup after DMA client has finished
From: Mason @ 2016-12-08 10:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208103921.GC6408@localhost>

On 08/12/2016 11:39, Vinod Koul wrote:

> On Wed, Dec 07, 2016 at 04:45:58PM +0000, M?ns Rullg?rd wrote:
>
>> Vinod Koul <vinod.koul@intel.com> writes:
>>
>>> On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>>>>
>>>> That's not going to work very well.  Device drivers typically request
>>>> dma channels in their probe functions or when the device is opened.
>>>> This means that reserving one of the few channels there will inevitably
>>>> make some other device fail to operate.
>>>
>>> No that doesn't make sense at all, you should get a channel only when you
>>> want to use it and not in probe!
>>
>> Tell that to just about every single driver ever written.
> 
> Not really, few do yes which is wrong but not _all_ do that.

Vinod,

Could you explain something to me in layman's terms?

I have a NAND Flash Controller driver that depends on the
DMA driver under discussion.

Suppose I move the dma_request_chan() call from the driver's
probe function, to the actual DMA transfer function.

I would want dma_request_chan() to put the calling thread
to sleep until a channel becomes available (possibly with
a timeout value).

But Maxime told me dma_request_chan() will just return
-EBUSY if no channels are available.

Am I supposed to busy wait in my driver's DMA function
until a channel becomes available?

I don't understand how the multiplexing of few memory
channels to many clients is supposed to happen efficiently?

Regards.

^ permalink raw reply

* [RFC PATCH v3 07/11] arm64: compat: Add a 32-bit vDSO
From: Kevin Brodsky @ 2016-12-08 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <87r35jmv3e.fsf@wedge.i-did-not-set--mail-host-address--so-tickle-me>

On 07/12/16 18:51, Nathan Lynch wrote:
> Hi Kevin,

Hi Nathan,

Thanks for taking a look at these patches!

> Kevin Brodsky <kevin.brodsky@arm.com> writes:
>> diff --git a/arch/arm64/kernel/vdso32/vgettimeofday.c b/arch/arm64/kernel/vdso32/vgettimeofday.c
>> new file mode 100644
>> index 000000000000..53c3d1f82b26
>> --- /dev/null
>> +++ b/arch/arm64/kernel/vdso32/vgettimeofday.c
> As I said at LPC last month, I'm not excited to have arch/arm's
> vgettimeofday.c copied into arch/arm64 and tweaked; I'd rather see this
> part of the implementation shared between arch/arm and arch/arm64
> somehow, even if there's not an elegant way to do so.
>
> The situation which must be avoided is one where the arch/arm64 compat
> VDSO incompatibly diverges from the arch/arm VDSO.  That becomes much
> less likely if there's only one copy of the userspace-exposed code to
> maintain.

I still agree this is very suboptimal. However, I also think this is by far the most 
straightforward solution, and I would like to stick to it *as a first step*. If you 
diff this "tweaked" vgettimeofday.c with the original one, you'll see that it is 
really not obvious to share even parts of vgettimeofday.c in the current state of arm 
and arm64. Besides, arm's vDSO hasn't changed much since it has been introduced last 
year, and vgettimeofday.c hasn't changed at all, so I think it's fair to say 
divergences are unlikely to happen in the near future.

Nevertheless, I completely agree this is not a good situation in the long run, and I 
am not happy with leaving things in this state. After this series is merged, my plan 
is to try and align arm and arm64 both in terms of vDSO features and kernel-vDSO 
interface. This would notably involve:
* Aligning the vDSO data struct's (especially vdso_data's member names), so that 
vgettimeofday.c can be (mostly) shared.
* Non-trivial Makefile work to share the same one between the arm and arm64 compat 
vDSOs. The refactoring I did in v3 is a first step, as not using top-level flags 
makes it more feasible to share the same Makefile.
* Adding support for CLOCK_MONOTONIC_RAW, as I did in the 64-bit vDSO.
* Moving arm's sigreturn trampolines to its vDSO (if enabled), and convince glibc to 
use the kernel's trampolines like on arm64.

This represents a substantial amount of work, which I think should be in a separate 
series (this one has already grown bigger than I expected).

The final step would be to investigate whether the 64-bit vDSO could reasonably move 
to a C implementation, and if it is the case unify the 3 vDSOs.

> [...]
>
>> +/*
>> + * We use the hidden visibility to prevent the compiler from generating a GOT
>> + * relocation. Not only is going through a GOT useless (the entry couldn't and
>> + * musn't be overridden by another library), it does not even work: the linker
>> + * cannot generate an absolute address to the data page.
>> + *
>> + * With the hidden visibility, the compiler simply generates a PC-relative
>> + * relocation (R_ARM_REL32), and this is what we need.
>> + */
>> +extern const struct vdso_data _vdso_data __attribute__((visibility("hidden")));
>> +
>> +static inline const struct vdso_data *get_vdso_data(void)
>> +{
>> +	const struct vdso_data *ret;
>> +	/*
>> +	 * This simply puts &_vdso_data into ret. The reason why we don't use
>> +	 * `ret = &_vdso_data` is that the compiler tends to optimise this in a
>> +	 * very suboptimal way: instead of keeping &_vdso_data in a register,
>> +	 * it goes through a relocation almost every time _vdso_data must be
>> +	 * accessed (even in subfunctions). This is both time and space
>> +	 * consuming: each relocation uses a word in the code section, and it
>> +	 * has to be loaded at runtime.
>> +	 *
>> +	 * This trick hides the assignment from the compiler. Since it cannot
>> +	 * track where the pointer comes from, it will only use one relocation
>> +	 * where get_vdso_data() is called, and then keep the result in a
>> +	 * register.
>> +	 */
>> +	asm("mov %0, %1" : "=r"(ret) : "r"(&_vdso_data));
>> +	return ret;
>> +}
> I think this is an instructive case: this code looks like an improvement
> over how the arch/arm VDSO accesses the data page.  Enhancements like
> this (not to mention fixes) shouldn't require a patch-per-architecture;
> things inevitably get out of sync and it's more of a maintenance burden
> in the long term.

What arm does is suboptimal indeed, as it uses a whole function to derive vdso_data's 
address instead of a simple relocation. However both methods are functionally 
equivalent. The approach I used here is simply consistent with how vdso_data is 
accessed in the 64-bit vDSO (with those two tricks because of both an assembler 
difference and the fact we are using C here, and GCC is not being smart). Again, I 
agree the arm vDSO should use the same approach, but unification requires other 
changes beyond the scope of this first series.

Thanks,
Kevin

^ permalink raw reply

* [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: PrasannaKumar Muralidharan @ 2016-12-08 11:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208092400.GA9743@Red>

>> The hwrng interface was always meant to be an interface for real
>> hardware random number generators.  People rely on that so we
>> should not provide bogus entropy sources through this interface.
>>
>
> Why not adding a KCONFIG HW_RANDOM_ACCEPT_ALSO_PRNG with big warning ?
> Or a HW_PRNG Kconfig which do the same than hwrandom with /dev/prng ?
> With that it will be much easier to convert in-tree PRNG that you want to remove.

I do have driver for a PRNG that I was planning to post in sometime.
Upon seeing this thread I think the code has to be changed.

I completely agree with Corentin, adding /dev/prng or /dev/hw_prng
will make it easier to move existing code. It can be made explicit
that using new device will provide only pseudo random number.

Thanks,
PrasannaKumar

^ permalink raw reply

* [Linaro-acpi] [PATCH v17 08/15] clocksource/drivers/arm_arch_timer: move arch_timer_needs_of_probing into DT init call
From: Mark Rutland @ 2016-12-08 11:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CADyBb7vAfMGV5ERFqAbwo4v4BhJv5s_-x4DXmdvy=iUacBzJWQ@mail.gmail.com>

On Thu, Dec 08, 2016 at 11:16:21AM +0800, Fu Wei wrote:
> Hi Timur,
> 
> On 8 December 2016 at 01:25, Timur Tabi <timur@codeaurora.org> wrote:
> > On Fri, Nov 25, 2016 at 9:06 AM, Fu Wei <fu.wei@linaro.org> wrote:
> >>
> >> a "+ int ret;" should be move from [12/15] to here, I have fix the
> >> problem in my repo, it would happen in next patchset
> >>
> >> https://git.linaro.org/people/fu.wei/linux.git/log/?h=topic-gtdt-wakeup-timer_upstream_v18_devel
> >
> > Fu, please post v18 to the mailing list so that it can be picked up
> > for 4.10 (if it's not too late already).

Unfortunately, it's too late for v4.10. It hasn't been sat in linux-next
at all, and we've seen kbuild test failures.

Hopefully there's time to beat this into shape and get it into
linux-next so that it's ready to queue for v4.11, though.

> Great thanks for your suggestion!  :-)
> yes, you are right, I would love to post v18 ASAP.
> 
> But I am still waiting for more feedback from the maintainers.

Please post a version which passes inspection by the kbuild test robot.
I haven't had a chance to look at this yet, and it'll be better to look
at a version that actually works.

Thanks,
Mark.

^ permalink raw reply

* [PATCH V8 1/3] ACPI: Add support for ResourceSource/IRQ domain mapping
From: Lorenzo Pieralisi @ 2016-12-08 11:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480460259-8585-2-git-send-email-agustinv@codeaurora.org>

Hi Agustin,

please CC me for next version.

On Tue, Nov 29, 2016 at 05:57:37PM -0500, Agustin Vega-Frias wrote:
> When an Extended IRQ Resource contains a valid ResourceSource
> use it to map the IRQ on the domain associated with the ACPI
> device referenced.
> 
> With this in place an irqchip driver can create its domain using
> irq_domain_create_linear and pass the device fwnode to create
> the domain mapping. When dependent devices are probed these
> changes allow the ACPI core find the domain and map the IRQ.
> 
> Signed-off-by: Agustin Vega-Frias <agustinv@codeaurora.org>
> ---
>  drivers/acpi/Makefile         |  2 +-
>  drivers/acpi/{gsi.c => irq.c} | 99 +++++++++++++++++++++++++++++++++++++------

[...]

> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> +static void acpi_dev_get_irqresource(struct resource *res, u32 hwirq,
> +				     struct fwnode_handle *source,
>  				     u8 triggering, u8 polarity, u8 shareable,
>  				     bool legacy)
>  {
>  	int irq, p, t;
>  
> -	if (!valid_IRQ(gsi)) {
> -		acpi_dev_irqresource_disabled(res, gsi);
> +	if (!source && !valid_IRQ(hwirq)) {

If you make source a struct acpi_resource_source pointer it could be:

if (source || !valid_IRQ(hwirq))

Actually we would not even need to pass the pointer, if we detect
an acpi_resource_source dependency we can just disable the resource
(without even looking-up the fwnode_handle, see below), it is a design
choice we have to make.

> +		acpi_dev_irqresource_disabled(res, hwirq);
>  		return;
>  	}
>  
> @@ -402,25 +403,25 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>  	 * using extended IRQ descriptors we take the IRQ configuration
>  	 * from _CRS directly.
>  	 */
> -	if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
> +	if (legacy && !acpi_get_override_irq(hwirq, &t, &p)) {
>  		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>  		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>  
>  		if (triggering != trig || polarity != pol) {
> -			pr_warning("ACPI: IRQ %d override to %s, %s\n", gsi,
> -				   t ? "level" : "edge", p ? "low" : "high");
> +			pr_warn("ACPI: IRQ %d override to %s, %s\n", hwirq,
> +				t ? "level" : "edge", p ? "low" : "high");
>  			triggering = trig;
>  			polarity = pol;
>  		}
>  	}
>  
>  	res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> -	irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> +	irq = acpi_register_irq(source, hwirq, triggering, polarity);
>  	if (irq >= 0) {
>  		res->start = irq;
>  		res->end = irq;
>  	} else {
> -		acpi_dev_irqresource_disabled(res, gsi);
> +		acpi_dev_irqresource_disabled(res, hwirq);
>  	}
>  }
>  
> @@ -448,6 +449,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  {
>  	struct acpi_resource_irq *irq;
>  	struct acpi_resource_extended_irq *ext_irq;
> +	struct fwnode_handle *src;
>  
>  	switch (ares->type) {
>  	case ACPI_RESOURCE_TYPE_IRQ:
> @@ -460,7 +462,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, irq->interrupts[index],
> +		acpi_dev_get_irqresource(res, irq->interrupts[index], NULL,
>  					 irq->triggering, irq->polarity,
>  					 irq->sharable, true);
>  		break;
> @@ -470,7 +472,8 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>  			acpi_dev_irqresource_disabled(res, 0);
>  			return false;
>  		}
> -		acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
> +		src = acpi_get_irq_source_fwhandle(&ext_irq->resource_source);

I think the only pending question on my side for this series is whether
we still carry out the domain look-up here (as you are doing now), or,
if we detect a resource_source dependency, we just disable the resource
and leave the deferred probing mechanism to deal with it, this will
completely decouple the current resource parsing from the deferred probe
mechanism that you are introducing; basically this is equivalent to
saying "if the IRQ resource has a dependency let's resolve it at
acpi_irq_get() time, not now".

I am fine either way, I just think that leaving the domain look-up
in the middle of the IRQ resource parsing is not really clean-cut.

Thanks,
Lorenzo

> +		acpi_dev_get_irqresource(res, ext_irq->interrupts[index], src,
>  					 ext_irq->triggering, ext_irq->polarity,
>  					 ext_irq->sharable, false);
>  		break;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 61a3d90..154e576 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -321,6 +321,25 @@ void acpi_set_irq_model(enum acpi_irq_model_id model,
>   */
>  void acpi_unregister_gsi (u32 gsi);
>  
> +#ifdef CONFIG_ACPI_GENERIC_GSI
> +struct fwnode_handle *
> +acpi_get_irq_source_fwhandle(const struct acpi_resource_source *source);
> +int acpi_register_irq(struct fwnode_handle *source, u32 hwirq, int trigger,
> +		      int polarity);
> +void acpi_unregister_irq(struct fwnode_handle *source, u32 hwirq);
> +#else
> +#define acpi_get_irq_source_fwhandle(source) (NULL)
> +static inline int acpi_register_irq(struct fwnode_handle *source, u32 hwirq,
> +				    int trigger, int polarity)
> +{
> +	return acpi_register_gsi(NULL, hwirq, trigger, polarity);
> +}
> +static inline void acpi_unregister_irq(struct fwnode_handle *source, u32 hwirq)
> +{
> +	acpi_unregister_gsi(hwirq);
> +}
> +#endif
> +
>  struct pci_dev;
>  
>  int acpi_pci_irq_enable (struct pci_dev *dev);
> -- 
> Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm Technologies, Inc.
> Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Geert Uytterhoeven @ 2016-12-08 11:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <91b0d10c-1bc2-c3e1-4088-f4ad9adcd6c0@free.fr>

On Thu, Dec 8, 2016 at 11:54 AM, Mason <slash.tmp@free.fr> wrote:
> On 08/12/2016 11:39, Vinod Koul wrote:
>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, M?ns Rullg?rd wrote:
>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>> On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>>>>> That's not going to work very well.  Device drivers typically request
>>>>> dma channels in their probe functions or when the device is opened.
>>>>> This means that reserving one of the few channels there will inevitably
>>>>> make some other device fail to operate.
>>>>
>>>> No that doesn't make sense at all, you should get a channel only when you
>>>> want to use it and not in probe!
>>>
>>> Tell that to just about every single driver ever written.
>>
>> Not really, few do yes which is wrong but not _all_ do that.
>
> Vinod,
>
> Could you explain something to me in layman's terms?
>
> I have a NAND Flash Controller driver that depends on the
> DMA driver under discussion.
>
> Suppose I move the dma_request_chan() call from the driver's
> probe function, to the actual DMA transfer function.
>
> I would want dma_request_chan() to put the calling thread
> to sleep until a channel becomes available (possibly with
> a timeout value).
>
> But Maxime told me dma_request_chan() will just return
> -EBUSY if no channels are available.
>
> Am I supposed to busy wait in my driver's DMA function
> until a channel becomes available?

Can you fall back to PIO if requesting a channel fails?

Alternatively, dma_request_chan() could always succeed, and
dmaengine_prep_slave_sg() could fail if the channel is currently not
available due to a limitation on the number of active channels, and
the driver could fall back to PIO for that transfer.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] arm64: Work around Falkor erratum 1009
From: Will Deacon @ 2016-12-08 11:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207200431.4587-1-cov@codeaurora.org>

On Wed, Dec 07, 2016 at 03:04:31PM -0500, Christopher Covington wrote:
> From: Shanker Donthineni <shankerd@codeaurora.org>
> 
> During a TLB invalidate sequence targeting the inner shareable
> domain, Falkor may prematurely complete the DSB before all loads
> and stores using the old translation are observed; instruction
> fetches are not subject to the conditions of this erratum.
> 
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  arch/arm64/Kconfig                | 10 +++++++++
>  arch/arm64/include/asm/cpucaps.h  |  3 ++-
>  arch/arm64/include/asm/tlbflush.h | 43 +++++++++++++++++++++++++++++++++++++++
>  arch/arm64/kernel/cpu_errata.c    |  7 +++++++
>  arch/arm64/kvm/hyp/tlb.c          | 39 ++++++++++++++++++++++++++++++-----
>  5 files changed, 96 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1004a3d..125440f 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -485,6 +485,16 @@ config QCOM_FALKOR_ERRATUM_E1003
>  
>  	  If unsure, say Y.
>  
> +config QCOM_FALKOR_ERRATUM_E1009
> +	bool "Falkor E1009: Prematurely complete a DSB after a TLBI"
> +	default y
> +	help
> +	  Falkor CPU may prematurely complete a DSB following a TLBI xxIS
> +	  invalidate maintenance operations. Repeat the TLBI operation one
> +	  more time to fix the issue.
> +
> +	  If unsure, say Y.

Call me perverse, but I like this workaround. People often tend to screw
up TLBI and DVM sync, but the IPI-based workaround is horribly invasive
and fragile. Simply repeating the operation tends to be enough to make
the chance of failure small enough to be acceptable.

> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> index cb6a8c2..5357d7f 100644
> --- a/arch/arm64/include/asm/cpucaps.h
> +++ b/arch/arm64/include/asm/cpucaps.h
> @@ -35,7 +35,8 @@
>  #define ARM64_HYP_OFFSET_LOW			14
>  #define ARM64_MISMATCHED_CACHE_LINE_SIZE	15
>  #define ARM64_WORKAROUND_QCOM_FALKOR_E1003	16
> +#define ARM64_WORKAROUND_QCOM_FALKOR_E1009	17

Could you rename this to something like ARM64_WORKAROUND_REPEAT_TLBI, so
that it could potentially be used by others?

>  
> -#define ARM64_NCAPS				17
> +#define ARM64_NCAPS				18
>  
>  #endif /* __ASM_CPUCAPS_H */
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index deab523..03bafc5 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -23,6 +23,7 @@
>  
>  #include <linux/sched.h>
>  #include <asm/cputype.h>
> +#include <asm/alternative.h>
>  
>  /*
>   * Raw TLBI operations.
> @@ -94,6 +95,13 @@ static inline void flush_tlb_all(void)
>  	dsb(ishst);
>  	__tlbi(vmalle1is);
>  	dsb(ish);
> +	asm volatile(ALTERNATIVE(
> +		     "nop \n"
> +		     "nop \n",
> +		     "tlbi vmalle1is \n"
> +		     "dsb ish \n",
> +		     ARM64_WORKAROUND_QCOM_FALKOR_E1009)
> +		     : :);

I'd much rather this was part of the __tlbi macro, which would hopefully
restrict this to one place in the code.

Will

^ permalink raw reply

* [Linaro-acpi] [PATCH v17 08/15] clocksource/drivers/arm_arch_timer: move arch_timer_needs_of_probing into DT init call
From: Fu Wei @ 2016-12-08 11:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208110410.GA9768@leverpostej>

Hi Mark,

On 8 December 2016 at 19:04, Mark Rutland <mark.rutland@arm.com> wrote:
> On Thu, Dec 08, 2016 at 11:16:21AM +0800, Fu Wei wrote:
>> Hi Timur,
>>
>> On 8 December 2016 at 01:25, Timur Tabi <timur@codeaurora.org> wrote:
>> > On Fri, Nov 25, 2016 at 9:06 AM, Fu Wei <fu.wei@linaro.org> wrote:
>> >>
>> >> a "+ int ret;" should be move from [12/15] to here, I have fix the
>> >> problem in my repo, it would happen in next patchset
>> >>
>> >> https://git.linaro.org/people/fu.wei/linux.git/log/?h=topic-gtdt-wakeup-timer_upstream_v18_devel
>> >
>> > Fu, please post v18 to the mailing list so that it can be picked up
>> > for 4.10 (if it's not too late already).
>
> Unfortunately, it's too late for v4.10. It hasn't been sat in linux-next
> at all, and we've seen kbuild test failures.
>
> Hopefully there's time to beat this into shape and get it into
> linux-next so that it's ready to queue for v4.11, though.

cross fingers for getting into v4.11 :-)

>
>> Great thanks for your suggestion!  :-)
>> yes, you are right, I would love to post v18 ASAP.
>>
>> But I am still waiting for more feedback from the maintainers.
>
> Please post a version which passes inspection by the kbuild test robot.
> I haven't had a chance to look at this yet, and it'll be better to look
> at a version that actually works.

OK, NP, will post them in several hours
Great thanks for your feedback.

>
> Thanks,
> Mark.



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

^ permalink raw reply

* [PATCH 2/3] arm64: Work around Falkor erratum 1003
From: Mark Rutland @ 2016-12-08 11:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207200028.4420-2-cov@codeaurora.org>

On Wed, Dec 07, 2016 at 03:00:26PM -0500, Christopher Covington wrote:
> From: Shanker Donthineni <shankerd@codeaurora.org>
> 
> On the Qualcomm Datacenter Technologies Falkor v1 CPU, memory accesses may
> allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being
> updated. Changing the TTBRx_EL1[ASID] and TTBRx_EL1[BADDR] fields
> separately using a reserved ASID will ensure that there are no TLB entries
> with incorrect ASID after changing the the ASID.
> 
> Pseudo code:
>   write TTBRx_EL1[ASID] to a reserved value
>   ISB
>   write TTBRx_EL1[BADDR] to a desired value
>   ISB
>   write TTBRx_EL1[ASID] to a desired value
>   ISB
> 
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  arch/arm64/Kconfig               | 11 +++++++++++
>  arch/arm64/include/asm/cpucaps.h |  3 ++-
>  arch/arm64/kernel/cpu_errata.c   |  7 +++++++
>  arch/arm64/mm/context.c          | 10 ++++++++++
>  arch/arm64/mm/proc.S             | 21 +++++++++++++++++++++
>  5 files changed, 51 insertions(+), 1 deletion(-)

This needs an update to Documentation/arm64/silicon-errata.txt.

> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index efcf1f7..f8d94ff 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -87,6 +87,11 @@ static void flush_context(unsigned int cpu)
>  	/* Update the list of reserved ASIDs and the ASID bitmap. */
>  	bitmap_clear(asid_map, 0, NUM_USER_ASIDS);
>  
> +	/* Reserve ASID '1' for Falkor erratum E1003 */
> +	if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_E1003) &&
> +	    cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
> +		__set_bit(1, asid_map);
> +
>  	/*
>  	 * Ensure the generation bump is observed before we xchg the
>  	 * active_asids.
> @@ -239,6 +244,11 @@ static int asids_init(void)
>  		panic("Failed to allocate bitmap for %lu ASIDs\n",
>  		      NUM_USER_ASIDS);
>  
> +	/* Reserve ASID '1' for Falkor erratum E1003 */
> +	if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_E1003) &&
> +	    cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
> +		__set_bit(1, asid_map);
> +
>  	pr_info("ASID allocator initialised with %lu entries\n", NUM_USER_ASIDS);
>  	return 0;
>  }
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 352c73b..b4d6508 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -134,6 +134,27 @@ ENDPROC(cpu_do_resume)
>  ENTRY(cpu_do_switch_mm)
>  	mmid	x1, x1				// get mm->context.id
>  	bfi	x0, x1, #48, #16		// set the ASID
> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_E1003
> +alternative_if_not ARM64_WORKAROUND_QCOM_FALKOR_E1003
> +	nop
> +	nop
> +	nop
> +	nop
> +	nop
> +	nop
> +	nop
> +	nop
> +alternative_else
> +	mrs     x2, ttbr0_el1                   // get cuurent TTBR0_EL1
> +	mov     x3, #1                          // reserved ASID

It might be best to define a FALCOR_E1003_RESERVED_ASID constant
somewhere, rather than using 1 directly here and in the ASID allocator.

> +	bfi     x2, x3, #48, #16                // set the reserved ASID + old BADDR
> +	msr     ttbr0_el1, x2                   // update TTBR0_EL1
> +	isb
> +	bfi     x2, x0, #0, #48                 // set the desired BADDR + reserved ASID
> +	msr     ttbr0_el1, x2                   // update TTBR0_EL1
> +	isb
> +alternative_endif

Please use alternative_if and alternative_else_nop_endif.

As Catalin noted, there are issues with stale and/or conflicting TLB
entries allocated with the reserved ASID, so we likely have to
invalidate that after the final switch.

Thanks,
Mark.

^ permalink raw reply

* [PATCH] arm64: Work around Falkor erratum 1009
From: Marc Zyngier @ 2016-12-08 11:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208112042.GB706@arm.com>

On 08/12/16 11:20, Will Deacon wrote:
> On Wed, Dec 07, 2016 at 03:04:31PM -0500, Christopher Covington wrote:
>> From: Shanker Donthineni <shankerd@codeaurora.org>
>>
>> During a TLB invalidate sequence targeting the inner shareable
>> domain, Falkor may prematurely complete the DSB before all loads
>> and stores using the old translation are observed; instruction
>> fetches are not subject to the conditions of this erratum.
>>
>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>> Signed-off-by: Christopher Covington <cov@codeaurora.org>
>> ---
>>  arch/arm64/Kconfig                | 10 +++++++++
>>  arch/arm64/include/asm/cpucaps.h  |  3 ++-
>>  arch/arm64/include/asm/tlbflush.h | 43 +++++++++++++++++++++++++++++++++++++++
>>  arch/arm64/kernel/cpu_errata.c    |  7 +++++++
>>  arch/arm64/kvm/hyp/tlb.c          | 39 ++++++++++++++++++++++++++++++-----
>>  5 files changed, 96 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 1004a3d..125440f 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -485,6 +485,16 @@ config QCOM_FALKOR_ERRATUM_E1003
>>  
>>  	  If unsure, say Y.
>>  
>> +config QCOM_FALKOR_ERRATUM_E1009
>> +	bool "Falkor E1009: Prematurely complete a DSB after a TLBI"
>> +	default y
>> +	help
>> +	  Falkor CPU may prematurely complete a DSB following a TLBI xxIS
>> +	  invalidate maintenance operations. Repeat the TLBI operation one
>> +	  more time to fix the issue.
>> +
>> +	  If unsure, say Y.
> 
> Call me perverse, but I like this workaround. People often tend to screw
> up TLBI and DVM sync, but the IPI-based workaround is horribly invasive
> and fragile. Simply repeating the operation tends to be enough to make
> the chance of failure small enough to be acceptable.
> 
>> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
>> index cb6a8c2..5357d7f 100644
>> --- a/arch/arm64/include/asm/cpucaps.h
>> +++ b/arch/arm64/include/asm/cpucaps.h
>> @@ -35,7 +35,8 @@
>>  #define ARM64_HYP_OFFSET_LOW			14
>>  #define ARM64_MISMATCHED_CACHE_LINE_SIZE	15
>>  #define ARM64_WORKAROUND_QCOM_FALKOR_E1003	16
>> +#define ARM64_WORKAROUND_QCOM_FALKOR_E1009	17
> 
> Could you rename this to something like ARM64_WORKAROUND_REPEAT_TLBI, so
> that it could potentially be used by others?

And add a parameter to it so that we can generate multiple TLBIs,
depending on the level of brokenness? ;-)

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH v5 2/5] i2c: Add STM32F4 I2C driver
From: kbuild test robot @ 2016-12-08 11:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481185563-8735-3-git-send-email-cedric.madianga@gmail.com>

Hi M'boumba,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.9-rc8 next-20161208]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/M-boumba-Cedric-Madianga/Add-support-for-the-STM32F4-I2C/20161208-173240
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: openrisc-allyesconfig (attached as .config)
compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=openrisc 

All warnings (new ones prefixed by >>):

   drivers/i2c/busses/i2c-stm32f4.c: In function 'stm32f4_i2c_set_periph_clk_freq':
>> drivers/i2c/busses/i2c-stm32f4.c:201:9: warning: comparison of distinct pointer types lacks a cast
>> drivers/i2c/busses/i2c-stm32f4.c:201:9: warning: comparison of distinct pointer types lacks a cast
>> drivers/i2c/busses/i2c-stm32f4.c:201:9: warning: comparison of distinct pointer types lacks a cast

vim +201 drivers/i2c/busses/i2c-stm32f4.c

   185	
   186	static void stm32f4_i2c_disable_it(struct stm32f4_i2c_dev *i2c_dev)
   187	{
   188		void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
   189	
   190		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
   191	}
   192	
   193	static void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
   194	{
   195		u32 clk_rate, cr2, freq;
   196	
   197		cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
   198		cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
   199		clk_rate = clk_get_rate(i2c_dev->clk);
   200		freq = clk_rate / MHZ_TO_HZ;
 > 201		freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
   202		cr2 |= STM32F4_I2C_CR2_FREQ(freq);
   203		writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
   204	}
   205	
   206	static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
   207	{
   208		u32 trise, freq, cr2, val;
   209	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 39528 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161208/970e614a/attachment-0001.gz>

^ permalink raw reply

* [GIT PULL] ARM: Xilinx Zynq SOC changes for v4.10
From: Michal Simek @ 2016-12-08 11:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207203629.GD10158@localhost>

On 7.12.2016 21:36, Olof Johansson wrote:
> On Tue, Dec 06, 2016 at 01:46:01PM +0100, Michal Simek wrote:
>> Hi,
>>
>> please pull this fix to your tree.
>>
>> Thanks,
>> Michal
>>
>>
>> The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
>>
>>   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
>>
>> are available in the git repository at:
>>
>>   https://github.com/Xilinx/linux-xlnx.git tags/zynq-soc-for-4.10
>>
>> for you to fetch changes up to 7a3cc2a7b2c723aa552028f4e66841cec183756d:
>>
>>   ARM: zynq: Reserve correct amount of non-DMA RAM (2016-11-14 16:07:58
>> +0100)
>>
>> ----------------------------------------------------------------
>> arm: Xilinx Zynq patches for v4.10
>>
>> - Fix dma issue
>>
>> ----------------------------------------------------------------
>> Kyle Roeschley (1):
>>       ARM: zynq: Reserve correct amount of non-DMA RAM
> 
> Merged, thanks.  Should this have been a fix, btw? I queued it for 4.10 merge
> window now.

yes it is also fix and should go to stable but it is not marked like
that why queue to 4.10 should be fine.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161208/dce0d59b/attachment.sig>

^ permalink raw reply

* [GIT PULL v2] ARM: Xilinx Zynq DT changes for v4.10
From: Michal Simek @ 2016-12-08 11:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207203919.GA11699@localhost>

On 7.12.2016 21:39, Olof Johansson wrote:
> On Tue, Dec 06, 2016 at 01:51:45PM +0100, Michal Simek wrote:
>> Hi,
>>
>> please add these changes to your arm-soc repo.
>> It fixes dtc warnings, small coding style changes and add support for
>> Microzed.
>>
>> Thanks,
>> Michal
>>
>> ---
>> v2: Use correct zynq-dt-for-4.10 tag instead of zynqmp-dt-for-4.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:
>>
>>   https://github.com/Xilinx/linux-xlnx.git tags/zynq-dt-for-4.10
>>
>> for you to fetch changes up to df2f3c48b9cd51e2612a1598342769d09d849f39:
>>
>>   arm: dts: zynq: Add MicroZed board support (2016-12-06 13:17:39 +0100)
> 
> Merged. In the future it'd be nice to get these a few weeks earlier in the
> cycle.

I know and sorry about it - busy time before Christmas.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161208/934e330c/attachment.sig>

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-12-08 11:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208103730.GB6408@localhost>

Vinod Koul <vinod.koul@intel.com> writes:

> On Wed, Dec 07, 2016 at 04:44:55PM +0000, M?ns Rullg?rd wrote:
>> Vinod Koul <vinod.koul@intel.com> writes:
>> >> 
>> >> What you're proposing, Vinod, is to make a channel exclusive
>> >> to a driver, as long as the driver has not explicitly released
>> >> the channel, via dma_release_channel(), right?
>> >
>> > Precisely, but yes the downside of that is concurrent access are
>> > limited, but am not sure if driver implements virtual channels and
>> > allows that..
>> 
>> My driver implements virtual channels.  The problem is that the physical
>> dma channels signal completion slightly too soon, at least with
>> mem-to-device transfers.  Apparently we need to keep the sbox routing
>> until the peripheral indicates that it has actually received all the
>> data.
>
> So do we need concurrent accesses by all clients.

Depends on what people do with the chips.

-- 
M?ns Rullg?rd

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-12-08 11:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161208103921.GC6408@localhost>

Vinod Koul <vinod.koul@intel.com> writes:

> On Wed, Dec 07, 2016 at 04:45:58PM +0000, M?ns Rullg?rd wrote:
>> Vinod Koul <vinod.koul@intel.com> writes:
>> 
>> > On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>> >> 
>> >> That's not going to work very well.  Device drivers typically request
>> >> dma channels in their probe functions or when the device is opened.
>> >> This means that reserving one of the few channels there will inevitably
>> >> make some other device fail to operate.
>> >
>> > No that doesnt make sense at all, you should get a channel only when you
>> > want to use it and not in probe!
>> 
>> Tell that to just about every single driver ever written.
>
> Not really, few do yes which is wrong but not _all_ do that.

Every driver I ever looked at does.  Name one you consider "correct."

-- 
M?ns Rullg?rd

^ permalink raw reply

* [PATCH] arm64: Work around Falkor erratum 1009
From: Mark Rutland @ 2016-12-08 11:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161207200431.4587-1-cov@codeaurora.org>

On Wed, Dec 07, 2016 at 03:04:31PM -0500, Christopher Covington wrote:
> From: Shanker Donthineni <shankerd@codeaurora.org>
> 
> During a TLB invalidate sequence targeting the inner shareable
> domain, Falkor may prematurely complete the DSB before all loads
> and stores using the old translation are observed; instruction
> fetches are not subject to the conditions of this erratum.
> 
> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
> Signed-off-by: Christopher Covington <cov@codeaurora.org>
> ---
>  arch/arm64/Kconfig                | 10 +++++++++
>  arch/arm64/include/asm/cpucaps.h  |  3 ++-
>  arch/arm64/include/asm/tlbflush.h | 43 +++++++++++++++++++++++++++++++++++++++
>  arch/arm64/kernel/cpu_errata.c    |  7 +++++++
>  arch/arm64/kvm/hyp/tlb.c          | 39 ++++++++++++++++++++++++++++++-----
>  5 files changed, 96 insertions(+), 6 deletions(-)

Please update Documentation/arm64/silicon-errata.txt respectively.

[...]

>  #include <linux/sched.h>
>  #include <asm/cputype.h>
> +#include <asm/alternative.h>

Nit: please keep includes (alphabetically) ordered (at least below the
linux/ or asm/ level).

[...]

> +	asm volatile(ALTERNATIVE(
> +		     "nop \n"
> +		     "nop \n",
> +		     "tlbi vmalle1is \n"
> +		     "dsb ish \n",

As a general note, perhaps we want a C compatible NOP_ALTERNATIVE() so
that the nop case can be implicitly generated for sequences like this.

Thanks,
Mark.

^ permalink raw reply

* [GIT PULL] efi: Pass secure boot mode to kernel
From: David Howells @ 2016-12-08 11:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Matt, Ard,

Is it too late to request this for the upcoming merge window?  Also, I've made
Lukas's requested changes and reposted just that patch in my reply to him.  Do
you want me to repost the lot?

Here's a set of patches that can determine the secure boot state of the
UEFI BIOS and pass that along to the main kernel image.  This involves
generalising ARM's efi_get_secureboot() function and making it mixed-mode
safe.

Changes:

 Ver 6:
  - Removed unnecessary variable init and trimmed comment.
  - Return efi_secureboot_mode_disabled directly rather than going to a
    place that just returns it.
  - Switched the last two patches.

 Ver 5:
  - Fix i386 compilation error (rsi should've been changed to esi).
  - Fix arm64 compilation error ('sys_table_arg' is a hidden macro parameter).

 Ver 4:
  - Use an enum to tell the kernel whether secure boot mode is enabled,
    disabled, couldn't be determined or wasn't even tried due to not being
    in EFI mode.
  - Support the UEFI-2.6 DeployedMode flag.
  - Don't clear boot_params->secure_boot in x86 sanitize_boot_params().
  - Preclear the boot_params->secure_boot on x86 head_*.S entry if we may
    not go through efi_main().

David
---
The following changes since commit 018edcfac4c3b140366ad51b0907f3becb5bb624:

  efi/libstub: Make efi_random_alloc() allocate below 4 GB on 32-bit (2016-11-25 07:15:23 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/efi-secure-boot-20161208

for you to fetch changes up to e71dd6bffca41faf7b4458c230e5c3d3c2b16d3e:

  efi: Add EFI_SECURE_BOOT bit (2016-12-08 08:19:04 +0000)

----------------------------------------------------------------
EFI secure boot

----------------------------------------------------------------
Ard Biesheuvel (1):
      efi: use typed function pointers for runtime services table

David Howells (5):
      x86/efi: Allow invocation of arbitrary runtime services
      arm/efi: Allow invocation of arbitrary runtime services
      efi: Add SHIM and image security database GUID definitions
      efi: Get the secure boot status
      efi: Handle secure boot from UEFI-2.6

Josh Boyer (2):
      efi: Disable secure boot if shim is in insecure mode
      efi: Add EFI_SECURE_BOOT bit

 Documentation/x86/zero-page.txt           |  2 +
 arch/arm/include/asm/efi.h                |  1 +
 arch/arm64/include/asm/efi.h              |  1 +
 arch/x86/boot/compressed/eboot.c          |  3 +
 arch/x86/boot/compressed/head_32.S        |  7 ++-
 arch/x86/boot/compressed/head_64.S        |  9 +--
 arch/x86/include/asm/bootparam_utils.h    |  5 +-
 arch/x86/include/asm/efi.h                |  5 ++
 arch/x86/include/uapi/asm/bootparam.h     |  3 +-
 arch/x86/kernel/asm-offsets.c             |  1 +
 arch/x86/kernel/setup.c                   | 15 +++++
 drivers/firmware/efi/libstub/Makefile     |  2 +-
 drivers/firmware/efi/libstub/arm-stub.c   | 63 ++------------------
 drivers/firmware/efi/libstub/secureboot.c | 99 +++++++++++++++++++++++++++++++
 include/linux/efi.h                       | 52 ++++++++++------
 15 files changed, 182 insertions(+), 86 deletions(-)
 create mode 100644 drivers/firmware/efi/libstub/secureboot.c

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-12-08 11:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdUULnPuTqEwRN2Z3gy2nmV37iLwJdNzrEvgpcG-EJS8OQ@mail.gmail.com>

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> On Thu, Dec 8, 2016 at 11:54 AM, Mason <slash.tmp@free.fr> wrote:
>> On 08/12/2016 11:39, Vinod Koul wrote:
>>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, M?ns Rullg?rd wrote:
>>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>>> On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>>>>>> That's not going to work very well.  Device drivers typically request
>>>>>> dma channels in their probe functions or when the device is opened.
>>>>>> This means that reserving one of the few channels there will inevitably
>>>>>> make some other device fail to operate.
>>>>>
>>>>> No that doesn't make sense at all, you should get a channel only when you
>>>>> want to use it and not in probe!
>>>>
>>>> Tell that to just about every single driver ever written.
>>>
>>> Not really, few do yes which is wrong but not _all_ do that.
>>
>> Vinod,
>>
>> Could you explain something to me in layman's terms?
>>
>> I have a NAND Flash Controller driver that depends on the
>> DMA driver under discussion.
>>
>> Suppose I move the dma_request_chan() call from the driver's
>> probe function, to the actual DMA transfer function.
>>
>> I would want dma_request_chan() to put the calling thread
>> to sleep until a channel becomes available (possibly with
>> a timeout value).
>>
>> But Maxime told me dma_request_chan() will just return
>> -EBUSY if no channels are available.
>>
>> Am I supposed to busy wait in my driver's DMA function
>> until a channel becomes available?
>
> Can you fall back to PIO if requesting a channel fails?
>
> Alternatively, dma_request_chan() could always succeed, and
> dmaengine_prep_slave_sg() could fail if the channel is currently not
> available due to a limitation on the number of active channels, and
> the driver could fall back to PIO for that transfer.

Why are we debating this nonsense?  There is an easy fix that doesn't
require changing the semantics of existing functions or falling back to
slow pio.

-- 
M?ns Rullg?rd

^ permalink raw reply

* [PATCH v9 05/11] arm/arm64: vgic: Introduce VENG0 and VENG1 fields to vmcr struct
From: Auger Eric @ 2016-12-08 11:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161128142836.GE18170@cbox>

Hi Vijay,

On 28/11/2016 15:28, Christoffer Dall wrote:
> On Wed, Nov 23, 2016 at 06:31:52PM +0530, vijay.kilari at gmail.com wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> ICC_VMCR_EL2 supports virtual access to ICC_IGRPEN1_EL1.Enable
>> and ICC_IGRPEN0_EL1.Enable fields. Add grpen0 and grpen1 member
>> variables to struct vmcr to support read and write of these fields.
>>
>> Also refactor vgic_set_vmcr and vgic_get_vmcr() code.
>> Drop ICH_VMCR_CTLR_SHIFT and ICH_VMCR_CTLR_MASK macros and instead
>> use ICH_VMCR_EOI* and ICH_VMCR_CBPR* macros
>> .
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>>  include/linux/irqchip/arm-gic-v3.h |  2 --
>>  virt/kvm/arm/vgic/vgic-mmio-v2.c   | 16 ----------------
>>  virt/kvm/arm/vgic/vgic-mmio.c      | 16 ++++++++++++++++
>>  virt/kvm/arm/vgic/vgic-v3.c        | 22 ++++++++++++++++++++--
>>  virt/kvm/arm/vgic/vgic.h           |  5 +++++
>>  5 files changed, 41 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
>> index b4f8287..406fc3e 100644
>> --- a/include/linux/irqchip/arm-gic-v3.h
>> +++ b/include/linux/irqchip/arm-gic-v3.h
>> @@ -404,8 +404,6 @@
>>  #define ICH_HCR_EN			(1 << 0)
>>  #define ICH_HCR_UIE			(1 << 1)
>>  
>> -#define ICH_VMCR_CTLR_SHIFT		0
>> -#define ICH_VMCR_CTLR_MASK		(0x21f << ICH_VMCR_CTLR_SHIFT)
>>  #define ICH_VMCR_CBPR_SHIFT		4
>>  #define ICH_VMCR_CBPR_MASK		(1 << ICH_VMCR_CBPR_SHIFT)
>>  #define ICH_VMCR_EOIM_SHIFT		9
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> index 2cb04b7..ad353b5 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> @@ -212,22 +212,6 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu,
>>  	}
>>  }
>>  
>> -static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
>> -{
>> -	if (kvm_vgic_global_state.type == VGIC_V2)
>> -		vgic_v2_set_vmcr(vcpu, vmcr);
>> -	else
>> -		vgic_v3_set_vmcr(vcpu, vmcr);
>> -}
>> -
>> -static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
>> -{
>> -	if (kvm_vgic_global_state.type == VGIC_V2)
>> -		vgic_v2_get_vmcr(vcpu, vmcr);
>> -	else
>> -		vgic_v3_get_vmcr(vcpu, vmcr);
>> -}
>> -
>>  #define GICC_ARCH_VERSION_V2	0x2
>>  
>>  /* These are for userland accesses only, there is no guest-facing emulation. */
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
>> index 0d1bc98..f81e0e5 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
>> @@ -416,6 +416,22 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev,
>>  	return -ENXIO;
>>  }
>>  
>> +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
>> +{
>> +	if (kvm_vgic_global_state.type == VGIC_V2)
>> +		vgic_v2_set_vmcr(vcpu, vmcr);
>> +	else
>> +		vgic_v3_set_vmcr(vcpu, vmcr);
>> +}
>> +
>> +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
>> +{
>> +	if (kvm_vgic_global_state.type == VGIC_V2)
>> +		vgic_v2_get_vmcr(vcpu, vmcr);
>> +	else
>> +		vgic_v3_get_vmcr(vcpu, vmcr);
>> +}
>> +
>>  /*
>>   * kvm_mmio_read_buf() returns a value in a format where it can be converted
>>   * to a byte array and be directly observed as the guest wanted it to appear
>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
>> index 9f0dae3..a3ff04b 100644
>> --- a/virt/kvm/arm/vgic/vgic-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-v3.c
>> @@ -175,10 +175,19 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp)
>>  {
>>  	u32 vmcr;
>>  
>> -	vmcr  = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & ICH_VMCR_CTLR_MASK;
>> +	/*
>> +	 * Ignore the FIQen bit, because GIC emulation always implies
>> +	 * SRE=1 which means the vFIQEn bit is also RES1.
>> +	 */
>> +	vmcr = (vmcrp->ctlr & ICC_CTLR_EL1_EOImode_MASK) >>
>> +		ICC_CTLR_EL1_EOImode_SHIFT;
>> +	vmcr = (vmcr << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK;
I am not able to understand why we use ICC_CTLR _*macros here? Please
could you explain it to me? Besides if we want to ignore the FIQen bit
can't we change the ICH_VMCR_CTLR_MASK value?

Thanks

Eric
> 
> Nit: I think this can be written more nicely as:
> 	vmcr = ((vmcrp->ctlr >> ICC_CTLR_EL1_EOImode_SHIFT)
> 		<< ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK;
> 
>> +	vmcr |= (vmcrp->ctlr << ICH_VMCR_CBPR_SHIFT) & ICH_VMCR_CBPR_MASK;
>>  	vmcr |= (vmcrp->abpr << ICH_VMCR_BPR1_SHIFT) & ICH_VMCR_BPR1_MASK;
>>  	vmcr |= (vmcrp->bpr << ICH_VMCR_BPR0_SHIFT) & ICH_VMCR_BPR0_MASK;
>>  	vmcr |= (vmcrp->pmr << ICH_VMCR_PMR_SHIFT) & ICH_VMCR_PMR_MASK;
>> +	vmcr |= (vmcrp->grpen0 << ICH_VMCR_ENG0_SHIFT) & ICH_VMCR_ENG0_MASK;
>> +	vmcr |= (vmcrp->grpen1 << ICH_VMCR_ENG1_SHIFT) & ICH_VMCR_ENG1_MASK;
>>  
>>  	vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr = vmcr;
>>  }
>> @@ -187,10 +196,19 @@ void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp)
>>  {
>>  	u32 vmcr = vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr;
>>  
>> -	vmcrp->ctlr = (vmcr & ICH_VMCR_CTLR_MASK) >> ICH_VMCR_CTLR_SHIFT;
>> +	/*
>> +	 * Ignore the FIQen bit, because GIC emulation always implies
>> +	 * SRE=1 which means the vFIQEn bit is also RES1.
>> +	 */
>> +	vmcrp->ctlr = (vmcr & ICH_VMCR_EOIM_MASK) >> ICH_VMCR_EOIM_SHIFT;
>> +	vmcrp->ctlr = (vmcrp->ctlr << ICC_CTLR_EL1_EOImode_SHIFT) &
>> +		       ICC_CTLR_EL1_EOImode_MASK;
> 
> similarly, this could be written as:
> 	vmcrp->ctlr = ((vmcr >> ICH_VMCR_EOIM_SHIFT) <<
> 			ICC_CTLR_EL1_EOImode_SHIFT) & ICC_CTLR_EL1_EOImode_MASK;
> 
>> +	vmcrp->ctlr |= (vmcr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT;
>>  	vmcrp->abpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT;
>>  	vmcrp->bpr  = (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT;
>>  	vmcrp->pmr  = (vmcr & ICH_VMCR_PMR_MASK) >> ICH_VMCR_PMR_SHIFT;
>> +	vmcrp->grpen0 = (vmcr & ICH_VMCR_ENG0_MASK) >> ICH_VMCR_ENG0_SHIFT;
>> +	vmcrp->grpen1 = (vmcr & ICH_VMCR_ENG1_MASK) >> ICH_VMCR_ENG1_SHIFT;
>>  }
>>  
>>  #define INITIAL_PENDBASER_VALUE						  \
>> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
>> index 91f58b2..9232791 100644
>> --- a/virt/kvm/arm/vgic/vgic.h
>> +++ b/virt/kvm/arm/vgic/vgic.h
>> @@ -78,6 +78,9 @@ struct vgic_vmcr {
>>  	u32	abpr;
>>  	u32	bpr;
>>  	u32	pmr;
>> +	/* Below member variable are valid only for GICv3 */
>> +	u32	grpen0;
>> +	u32	grpen1;
>>  };
>>  
>>  struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
>> @@ -138,6 +141,8 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>>  int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>>  			 int offset, u32 *val);
>>  int kvm_register_vgic_device(unsigned long type);
>> +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
>> +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
>>  int vgic_lazy_init(struct kvm *kvm);
>>  int vgic_init(struct kvm *kvm);
>>  
>> -- 
>> 1.9.1
>>
> My comments on style above notwithstanding:
> 
> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply

* [GIT PULL] efi: Pass secure boot mode to kernel
From: Matt Fleming @ 2016-12-08 11:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4561.1481197517@warthog.procyon.org.uk>

On Thu, 08 Dec, at 11:45:17AM, David Howells wrote:
> Hi Matt, Ard,
> 
> Is it too late to request this for the upcoming merge window?

For something as non-trivial as this, yes, it's too late. We generally
close the EFI tree window for new features around -rc5 time.

> Also, I've made
> Lukas's requested changes and reposted just that patch in my reply to him.  Do
> you want me to repost the lot?
 
Please do, yeah.

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Geert Uytterhoeven @ 2016-12-08 11:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <yw1x60mu1w7e.fsf@unicorn.mansr.com>

On Thu, Dec 8, 2016 at 12:44 PM, M?ns Rullg?rd <mans@mansr.com> wrote:
> Vinod Koul <vinod.koul@intel.com> writes:
>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, M?ns Rullg?rd wrote:
>>> Vinod Koul <vinod.koul@intel.com> writes:
>>> > On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>>> >> That's not going to work very well.  Device drivers typically request
>>> >> dma channels in their probe functions or when the device is opened.
>>> >> This means that reserving one of the few channels there will inevitably
>>> >> make some other device fail to operate.
>>> >
>>> > No that doesnt make sense at all, you should get a channel only when you
>>> > want to use it and not in probe!
>>>
>>> Tell that to just about every single driver ever written.
>>
>> Not really, few do yes which is wrong but not _all_ do that.
>
> Every driver I ever looked at does.  Name one you consider "correct."

I'm far from claiming that drivers/tty/serial/sh-sci.c is perfect, but it does
request DMA channels at open time, not at probe time.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Geert Uytterhoeven @ 2016-12-08 12:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <yw1x1sxi1w3i.fsf@unicorn.mansr.com>

Hi M?ns,

On Thu, Dec 8, 2016 at 12:47 PM, M?ns Rullg?rd <mans@mansr.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> On Thu, Dec 8, 2016 at 11:54 AM, Mason <slash.tmp@free.fr> wrote:
>>> On 08/12/2016 11:39, Vinod Koul wrote:
>>>> On Wed, Dec 07, 2016 at 04:45:58PM +0000, M?ns Rullg?rd wrote:
>>>>> Vinod Koul <vinod.koul@intel.com> writes:
>>>>>> On Tue, Dec 06, 2016 at 01:14:20PM +0000, M?ns Rullg?rd wrote:
>>>>>>> That's not going to work very well.  Device drivers typically request
>>>>>>> dma channels in their probe functions or when the device is opened.
>>>>>>> This means that reserving one of the few channels there will inevitably
>>>>>>> make some other device fail to operate.
>>>>>>
>>>>>> No that doesn't make sense at all, you should get a channel only when you
>>>>>> want to use it and not in probe!
>>>>>
>>>>> Tell that to just about every single driver ever written.
>>>>
>>>> Not really, few do yes which is wrong but not _all_ do that.
>>>
>>> Vinod,
>>>
>>> Could you explain something to me in layman's terms?
>>>
>>> I have a NAND Flash Controller driver that depends on the
>>> DMA driver under discussion.
>>>
>>> Suppose I move the dma_request_chan() call from the driver's
>>> probe function, to the actual DMA transfer function.
>>>
>>> I would want dma_request_chan() to put the calling thread
>>> to sleep until a channel becomes available (possibly with
>>> a timeout value).
>>>
>>> But Maxime told me dma_request_chan() will just return
>>> -EBUSY if no channels are available.
>>>
>>> Am I supposed to busy wait in my driver's DMA function
>>> until a channel becomes available?
>>
>> Can you fall back to PIO if requesting a channel fails?
>>
>> Alternatively, dma_request_chan() could always succeed, and
>> dmaengine_prep_slave_sg() could fail if the channel is currently not
>> available due to a limitation on the number of active channels, and
>> the driver could fall back to PIO for that transfer.
>
> Why are we debating this nonsense?  There is an easy fix that doesn't
> require changing the semantics of existing functions or falling back to
> slow pio.

You still want to fall back to PIO if the DMA engine is not available at all
(e.g. DMA engine driver not compiled in, or module not loaded).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v3 4/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LHC)
From: Andrew Jeffery @ 2016-12-08 12:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACPK8XfuZ14Ud3Kginut7f0-_-UjTB-=Pma-9WwNecF93k0Ktg@mail.gmail.com>

On Thu, 2016-12-08 at 12:42 +1030, Joel Stanley wrote:
> > On Tue, Dec 6, 2016 at 1:23 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> > The LPC bus pinmux configuration on fifth generation Aspeed SoCs depends
> > on bits in both the System Control Unit and the LPC Host Controller.
> > 
> > The Aspeed LPC Host Controller is described as a child node of the
> > LPC host-range syscon device for arbitration of access by the host
> > controller and pinmux drivers.
> > 
> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> > ?.../devicetree/bindings/mfd/aspeed-lpc.txt?????????| 22 ++++++++++++++++++++++
> > ?1 file changed, 22 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> > index a97131aba446..9de318ef72da 100644
> > --- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> > +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> > > > @@ -109,3 +109,25 @@ lpc: lpc at 1e789000 {
> > ????????};
> > ?};
> > 
> > +Host Node Children
> > +==================
> > +
> > +LPC Host Controller
> > +-------------------
> > +
> > +The Aspeed LPC Host Controller configures the Low Pin Count (LPC) bus behaviour
> > +between the host and the baseboard management controller. The registers exist
> > +in the "host" portion of the Aspeed LPC controller, which must be the parent of
> > +the LPC host controller node.
> > +
> > +Required properties:
> > +- compatible:??????????"aspeed,ast2500-lhc";
> 
> Can you remind me why this binding doesn't cover the ast2400?

Partly that we haven't yet needed the LHC for the AST2400.

Mostly that I overlooked it.

If there are other problems with series I'll address this issue, but if
not we can add it when we need it down the track.

Andrew

> 
> Cheers,
> 
> Joel
> 
> > +- reg:?????????????????contains offset/length value of the LHC memory
> > +???????????????????????region.
> > +
> > +Example:
> > +
> > > > +lhc: lhc at 20 {
> > +???????compatible = "aspeed,ast2500-lhc";
> > +???????reg = <0x20 0x24 0x48 0x8>;
> > +};
> > --
> > 2.9.3
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161208/08062be4/attachment.sig>

^ permalink raw reply

* Tearing down DMA transfer setup after DMA client has finished
From: Mason @ 2016-12-08 12:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMuHMdWXVW9BG=MZGptibDTxDd5=7iCbiq1cmbD+qu-CZ0Y_7g@mail.gmail.com>

On 08/12/2016 13:03, Geert Uytterhoeven wrote:

> M?ns Rullg?rd wrote:
>
>> Geert Uytterhoeven writes:
>>
>>> Can you fall back to PIO if requesting a channel fails?
>>
>> Why are we debating this nonsense?  There is an easy fix that doesn't
>> require changing the semantics of existing functions or falling back to
>> slow pio.
> 
> You still want to fall back to PIO if the DMA engine is not available
> at all (e.g. DMA engine driver not compiled in, or module not loaded).

FWIW, the ECC engine is tied to the DMA engine. So PIO means
not only taking a hit from tying up the CPU for a slooow
transfer, but also a huge hit if ECC must be computed in SW.
(A 100x perf degradation is not unlikely.)

Regards.

^ permalink raw reply

* [PATCH v5 2/5] i2c: Add STM32F4 I2C driver
From: kbuild test robot @ 2016-12-08 12:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1481185563-8735-3-git-send-email-cedric.madianga@gmail.com>

Hi M'boumba,

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.9-rc8 next-20161208]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/M-boumba-Cedric-Madianga/Add-support-for-the-STM32F4-I2C/20161208-173240
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/compiler.h:253:8: sparse: attribute 'no_sanitize_address': unknown attribute
>> drivers/i2c/busses/i2c-stm32f4.c:201:16: sparse: incompatible types in comparison expression (different signedness)
>> drivers/i2c/busses/i2c-stm32f4.c:201:16: sparse: incompatible types in comparison expression (different signedness)
>> drivers/i2c/busses/i2c-stm32f4.c:201:16: sparse: incompatible types in comparison expression (different signedness)
   In file included from include/linux/clk.h:16:0,
                    from drivers/i2c/busses/i2c-stm32f4.c:12:
   drivers/i2c/busses/i2c-stm32f4.c: In function 'stm32f4_i2c_set_periph_clk_freq':
   include/linux/kernel.h:749:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&max1 == &max2);   \
                   ^
   include/linux/kernel.h:737:2: note: in definition of macro '__min'
     t1 min1 = (x);     \
     ^~
   include/linux/kernel.h:778:28: note: in expansion of macro 'min'
    #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
                               ^~~
   include/linux/kernel.h:752:2: note: in expansion of macro '__max'
     __max(typeof(x), typeof(y),   \
     ^~~~~
   include/linux/kernel.h:778:45: note: in expansion of macro 'max'
    #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
                                                ^~~
   drivers/i2c/busses/i2c-stm32f4.c:201:9: note: in expansion of macro 'clamp'
     freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
            ^~~~~
   include/linux/kernel.h:749:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&max1 == &max2);   \
                   ^
   include/linux/kernel.h:737:13: note: in definition of macro '__min'
     t1 min1 = (x);     \
                ^
   include/linux/kernel.h:778:28: note: in expansion of macro 'min'
    #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
                               ^~~
   include/linux/kernel.h:752:2: note: in expansion of macro '__max'
     __max(typeof(x), typeof(y),   \
     ^~~~~
   include/linux/kernel.h:778:45: note: in expansion of macro 'max'
    #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
                                                ^~~
   drivers/i2c/busses/i2c-stm32f4.c:201:9: note: in expansion of macro 'clamp'
     freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
            ^~~~~
   include/linux/kernel.h:739:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&min1 == &min2);   \
                   ^
   include/linux/kernel.h:742:2: note: in expansion of macro '__min'
     __min(typeof(x), typeof(y),   \
     ^~~~~
   include/linux/kernel.h:778:28: note: in expansion of macro 'min'
    #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
                               ^~~
   drivers/i2c/busses/i2c-stm32f4.c:201:9: note: in expansion of macro 'clamp'
     freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
            ^~~~~

vim +201 drivers/i2c/busses/i2c-stm32f4.c

   185	
   186	static void stm32f4_i2c_disable_it(struct stm32f4_i2c_dev *i2c_dev)
   187	{
   188		void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
   189	
   190		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
   191	}
   192	
   193	static void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
   194	{
   195		u32 clk_rate, cr2, freq;
   196	
   197		cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
   198		cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
   199		clk_rate = clk_get_rate(i2c_dev->clk);
   200		freq = clk_rate / MHZ_TO_HZ;
 > 201		freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
   202		cr2 |= STM32F4_I2C_CR2_FREQ(freq);
   203		writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
   204	}
   205	
   206	static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
   207	{
   208		u32 trise, freq, cr2, val;
   209	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply


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