Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] arm64: alternatives: Work around NOP generation with broken assembler
From: Marc Zyngier @ 2016-12-05 11:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c7de1968-7d50-a435-56ea-a7f3e9cb52da@arm.com>

On 05/12/16 10:58, Marc Zyngier wrote:
> On 05/12/16 10:05, Will Deacon wrote:
>> On Sat, Dec 03, 2016 at 02:05:38PM +0000, Marc Zyngier wrote:
>>> When compiling a .inst directive in an alternative clause with
>>> a rather old version of gas (circa 2.24), and when used with
>>> the alternative_else_nop_endif feature, the compilation fails
>>> with a rather cryptic message such as:
>>>
>>> arch/arm64/lib/clear_user.S:33: Error: bad or irreducible absolute expression
>>>
>>> which is caused by the bug described in eb7c11ee3c5c ("arm64:
>>> alternative: Work around .inst assembler bugs").
>>>
>>> This effectively prevents the use of the "nops" macro, which
>>> requires the number of instruction as a parameter (the assembler
>>> is confused and unable to compute the difference between two labels).
>>>
>>> As an alternative(!), use the .fill directive to output the number
>>> of required NOPs (.fill has the good idea to output the fill pattern
>>> in the endianness of the instruction stream).
>>
>> Are you sure about that? The gas docs say:
>>
>> `The contents of each repeat bytes is taken from an 8-byte number. The
>>  highest order 4 bytes are zero. The lowest order 4 bytes are value rendered
>>  in the byte-order of an integer on the computer as is assembling for.'
>>
>> and I'd expect "integer" to refer to data values, rather than instructions.
> 
> My tests on 2.24 and 2.25 seem to show that the output is always LE,
> which could be another GAS issue. I've asked the binutils people for
> information.

Well, my testing was wrong, as objdump -d was lying to me by displaying
something in little-endian form. Time for some more head scratching.

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

^ permalink raw reply

* [PATCH 1/3] ARM: dts: imx6: Add Savageboard common file
From: Fabio Estevam @ 2016-12-05 11:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205010729.7047-2-woogyom.kim@gmail.com>

On Sun, Dec 4, 2016 at 11:07 PM, Milo Kim <woogyom.kim@gmail.com> wrote:

> +       regulators {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               reg_3p3v: regulator at 0 {
> +                       compatible = "regulator-fixed";
> +                       reg = <0>;
> +                       regulator-name = "3P3V";
> +                       regulator-min-microvolt = <3300000>;
> +                       regulator-max-microvolt = <3300000>;
> +                       regulator-always-on;
> +               };

Please remove the regulators container and put the regulator node
directly as follows:

reg_3p3v: regulator-3p3v {
   compatible = "regulator-fixed";
   regulator-name = "3P3V";
   regulator-min-microvolt = <3300000>;
   regulator-max-microvolt = <3300000>;
   regulator-always-on;
}

> +       };
> +};
> +
> +&clks {
> +       assigned-clocks = <&clks IMX6QDL_CLK_LDB_DI0_SEL>,
> +                         <&clks IMX6QDL_CLK_LDB_DI1_SEL>;
> +       assigned-clock-parents = <&clks IMX6QDL_CLK_PLL3_USB_OTG>,
> +                                <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
> +};
> +
> +&fec {
> +       phy-mode = "rgmii";
> +       phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_HIGH>;

I think you meant
phy-reset-gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;

> +&iomuxc {
> +       savageboard {
> +               pinctrl_emmc: emmcgrp {
> +                       fsl,pins = <
> +                               MX6QDL_PAD_SD4_CMD__SD4_CMD             0x17059
> +                               MX6QDL_PAD_SD4_CLK__SD4_CLK             0x10059
> +                               MX6QDL_PAD_SD4_DAT0__SD4_DATA0          0x17059
> +                               MX6QDL_PAD_SD4_DAT1__SD4_DATA1          0x17059
> +                               MX6QDL_PAD_SD4_DAT2__SD4_DATA2          0x17059
> +                               MX6QDL_PAD_SD4_DAT3__SD4_DATA3          0x17059
> +                               MX6QDL_PAD_SD4_DAT4__SD4_DATA4          0x17059
> +                               MX6QDL_PAD_SD4_DAT5__SD4_DATA5          0x17059
> +                               MX6QDL_PAD_SD4_DAT6__SD4_DATA6          0x17059
> +                               MX6QDL_PAD_SD4_DAT7__SD4_DATA7          0x17059
> +                       >;
> +               };

You can remove the savegeboard level. Please check
arch/arm/boot/dts/imx6q-tbs2910.dts.

iomux usually go as the last node of the dts file.

^ permalink raw reply

* [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm
From: Thierry Reding @ 2016-12-05 11:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CA+M3ks7_utdw1c0A0m+RzuPTfvFAv5d8HoQvQDzXwHR8M6RKJA@mail.gmail.com>

On Mon, Dec 05, 2016 at 12:02:45PM +0100, Benjamin Gaignard wrote:
> 2016-12-05 8:23 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> > On Fri, Dec 02, 2016 at 11:17:19AM +0100, Benjamin Gaignard wrote:
> >> This driver add support for pwm driver on stm32 platform.
> >
> > "adds". Also please use PWM in prose because it's an abbreviation.
> >
> >> The SoC have multiple instances of the hardware IP and each
> >> of them could have small differences: number of channels,
> >> complementary output, counter register size...
> >> Use DT parameters to handle those differentes configuration
> >
> > "different configurations"
> 
> ok
> 
> >
> >>
> >> version 2:
> >> - only keep one comptatible
> >> - use DT paramaters to discover hardware block configuration
> >
> > "parameters"
> 
> ok
> 
> >
> >>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> ---
> >>  drivers/pwm/Kconfig     |   8 ++
> >>  drivers/pwm/Makefile    |   1 +
> >>  drivers/pwm/pwm-stm32.c | 285 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 294 insertions(+)
> >>  create mode 100644 drivers/pwm/pwm-stm32.c
> >>
> >> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> >> index bf01288..a89fdba 100644
> >> --- a/drivers/pwm/Kconfig
> >> +++ b/drivers/pwm/Kconfig
> >> @@ -388,6 +388,14 @@ config PWM_STI
> >>         To compile this driver as a module, choose M here: the module
> >>         will be called pwm-sti.
> >>
> >> +config PWM_STM32
> >> +     bool "STMicroelectronics STM32 PWM"
> >> +     depends on ARCH_STM32
> >> +     depends on OF
> >> +     select MFD_STM32_GP_TIMER
> >
> > Should that be a "depends on"?
> 
> I think select is fine because the wanted feature is PWM not the mfd part

I think in that case you may want to hide the MFD Kconfig option. See
Documentation/kbuild/kconfig-language.txt (notes about select) for the
details.

[...]
> >> +};
> >> +
> >> +#define to_stm32_pwm_dev(x) container_of(chip, struct stm32_pwm_dev, chip)
> >
> > Please turn this into a static inline.
> 
> with putting struct pwm_chip as first filed I will just cast the structure

Don't do that, please. container_of() is still preferred because it is
correct and won't break even if you (or somebody else) changes the order
in the future. The fact that it might be optimized away is a detail, or
a micro-optimization. Force-casting is a bad idea because it won't catch
errors if for some reason the field doesn't remain in the first position
forever.

> >> +static void stm32_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >> +{
> >> +     u32 mask;
> >> +     struct stm32_pwm_dev *dev = to_stm32_pwm_dev(chip);
> >> +
> >> +     /* Disable channel */
> >> +     mask = TIM_CCER_CC1E << (pwm->hwpwm * 4);
> >> +     if (dev->caps & CAP_COMPLEMENTARY)
> >> +             mask |= TIM_CCER_CC1NE << (pwm->hwpwm * 4);
> >> +
> >> +     regmap_update_bits(dev->regmap, TIM_CCER, mask, 0);
> >> +
> >> +     /* When all channels are disabled, we can disable the controller */
> >> +     if (!__active_channels(dev))
> >> +             regmap_update_bits(dev->regmap, TIM_CR1, TIM_CR1_CEN, 0);
> >> +
> >> +     clk_disable(dev->clk);
> >> +}
> >
> > All of the above can be folded into the ->apply() hook for atomic PWM
> > support.
> >
> > Also, in the above you use clk_enable() in the ->enable() callback and
> > clk_disable() in ->disable(). If you need the clock to access registers
> > you'll have to enabled it in the others as well because there are no
> > guarantees that configuration will only happen between ->enable() and
> > ->disable(). Atomic PWM simplifies this a bit, but you still need to be
> > careful about when to enable the clock in your ->apply() hook.
> 
> I have used regmap functions enable/disable clk for me when accessing to
> the registers.
> I only have to take care of clk enable/disable when PWM state change.

Okay, that's fine then.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161205/79897d78/attachment.sig>

^ permalink raw reply

* [PATCH 1/1] arm64: Correcting format specifier for printing 64 bit addresses
From: Will Deacon @ 2016-12-05 11:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480925393-8386-1-git-send-email-maninder1.s@samsung.com>

On Mon, Dec 05, 2016 at 01:39:53PM +0530, Maninder Singh wrote:
> This patch corrects format specifier for printing 64 bit addresses.
> 
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> ---
>  arch/arm64/kernel/signal.c |  2 +-
>  arch/arm64/kvm/sys_regs.c  |  8 ++++++--
>  arch/arm64/mm/fault.c      | 15 ++++++++++-----
>  arch/arm64/mm/mmu.c        |  4 ++--
>  4 files changed, 19 insertions(+), 10 deletions(-)

Any reason not to fix kvm/trace.h too?

Anyway, rest of this looks fine:

Acked-by: Will Deacon <will.deacon@arm.com>

Will

^ permalink raw reply

* [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings
From: Thierry Reding @ 2016-12-05 11:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CA+M3ks4-JQNGYtdiXNj1J701x0njKD0_bf4yN1mLSjEcT2Z=6A@mail.gmail.com>

On Mon, Dec 05, 2016 at 12:08:32PM +0100, Benjamin Gaignard wrote:
> 2016-12-05 7:53 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> > On Fri, Dec 02, 2016 at 11:17:18AM +0100, Benjamin Gaignard wrote:
> >> Define bindings for pwm-stm32
> >>
> >> version 2:
> >> - use parameters instead of compatible of handle the hardware configuration
> >>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> ---
> >>  .../devicetree/bindings/pwm/pwm-stm32.txt          | 38 ++++++++++++++++++++++
> >>  1 file changed, 38 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> new file mode 100644
> >> index 0000000..575b9fb
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> @@ -0,0 +1,38 @@
> >> +STMicroelectronics PWM driver bindings for STM32
> >
> > Technically this bindings describe devices, so "driver binding" is a
> > somewhat odd wording. Perhaps:
> >
> >         STMicroelectronics STM32 General Purpose Timer PWM bindings
> >
> > ?
>  done
> 
> >
> >> +
> >> +Must be a sub-node of STM32 general purpose timer driver
> >> +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
> >
> > Again, "driver parent node" is odd. Perhaps:
> >
> >         Must be a sub-node of an STM32 General Purpose Timer device tree
> >         node. See ../mfd/stm32-general-purpose-timer.txt for details about
> >         the parent node.
> >
> > ?
> 
> done
> 
> >
> >> +Required parameters:
> >> +- compatible:                Must be "st,stm32-pwm"
> >> +- pinctrl-names:     Set to "default".
> >> +- pinctrl-0:                 List of phandles pointing to pin configuration nodes
> >> +                     for PWM module.
> >> +                     For Pinctrl properties, please refer to [1].
> >
> > Your indentation and capitalization are inconsistent. Also, please refer
> > to the pinctrl bindings by relative path and inline, rather than as a
> > footnote reference.
> 
> OK
> 
> >
> >> +
> >> +Optional parameters:
> >> +- st,breakinput:     Set if the hardware have break input capabilities
> >> +- st,breakinput-polarity: Set break input polarity. Default is 0
> >> +                      The value define the active polarity:
> >> +                       - 0 (active LOW)
> >> +                       - 1 (active HIGH)
> >
> > Could we fold these into a single property? If st,breakinput-polarity is
> > not present it could simply mean that there is no break input, and if it
> > is present you don't have to rely on a default.
> 
> I need to know if I have to activate breakinput feature and on which level
> I will rewrite it like that:
> Optional parameters:
> - st,breakinput-polarity-high: Set if break input polarity is active
> on high level.
> - st,breakinput-polarity-high: Set if break input polarity is active
> on low level.

How is that different from a single property:

	Optional properties:
	- st,breakinput-polarity: If present, a break input is available
	    for the channel. In that case the property value denotes the
	    polarity of the break input:
	    - 0: active low
	    - 1: active high

?

> > The pwm- prefix is rather redundant since the node is already named pwm.
> > Why not simply st,channels? Or simply channels, since it's not really
> > anything specific to this hardware.
> >
> > Come to think of it, might be worth having a discussion with our DT
> > gurus about what their stance is on using the # as prefix for numbers
> > (such as in #address-cells or #size-cells). This could be #channels to
> > mark it more explicitly as representing a count.
> >
> >> +- st,32bits-counter: Set if the hardware have a 32 bits counter
> >> +- st,complementary:  Set if the hardware have complementary output channels
> >
> > "hardware has" and also maybe mention explicitly that this is a boolean
> > property. Otherwise people might be left wondering what it should be set
> > to. Or maybe word this differently to imply that it's boolean:
> >
> >         - st,32bits-counter: if present, the hardware has a 32 bit counter
> >         - st,complementary: if present, the hardware has a complementary
> >                             output channel
> 
> I found a way to detect, at probe time, the number of channels, counter size,
> break input capability and complementary channels so I will remove
> "st,breakinput", "st,32bits-counter", "st,complementary" and "st,pwm-num-chan"
> parameters

Oh hey, that's very neat. I suppose in that case my comment above about
the break input polarity is somewhat obsoleted. Still I think you won't
need two properties. Instead you can follow what other similar
properties have done: choose a default (often low-active) and have a
single optional property to override the default (often high-active).

In your case:

	- st,breakinput-active-high: Some channels have a break input,
	    whose polarity will be active low by default. If this
	    property is present, the channel will be configured with an
	    active high polarity for the break input.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161205/85093ef8/attachment.sig>

^ permalink raw reply

* next-20161205 build: 3 failures 4 warnings (next-20161205)
From: Mark Brown @ 2016-12-05 11:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <E1cDo86-0008IX-9W@optimist>

On Mon, Dec 05, 2016 at 07:56:06AM +0000, Build bot for Mark Brown wrote:

Today's -next fails to build an arm64 allnodconfig and allmodconfig
with:

> 	arm64-allnoconfig
> ../arch/arm64/lib/clear_user.S:33: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/clear_user.S:53: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/clear_user.S:33: Error: attempt to move .org backwards
> ../arch/arm64/lib/clear_user.S:53: Error: attempt to move .org backwards
> ../arch/arm64/lib/copy_from_user.S:67: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/copy_from_user.S:70: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/copy_from_user.S:67: Error: attempt to move .org backwards
> ../arch/arm64/lib/copy_from_user.S:70: Error: attempt to move .org backwards
> ../arch/arm64/lib/copy_in_user.S:68: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/copy_in_user.S:71: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/copy_in_user.S:68: Error: attempt to move .org backwards
> ../arch/arm64/lib/copy_in_user.S:71: Error: attempt to move .org backwards
> ../arch/arm64/lib/copy_to_user.S:66: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/copy_to_user.S:69: Error: bad or irreducible absolute expression
> ../arch/arm64/lib/copy_to_user.S:66: Error: attempt to move .org backwards
> ../arch/arm64/lib/copy_to_user.S:69: Error: attempt to move .org backwards

This was triggered somehow by bca8f17f57bd7 (arm64: Get rid of
asm/opcodes.h) though I didn't figure out how.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161205/cd10bb7a/attachment-0001.sig>

^ permalink raw reply

* [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings
From: Benjamin Gaignard @ 2016-12-05 11:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205065350.GA18069@ulmo.ba.sec>

2016-12-05 7:53 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> On Fri, Dec 02, 2016 at 11:17:18AM +0100, Benjamin Gaignard wrote:
>> Define bindings for pwm-stm32
>>
>> version 2:
>> - use parameters instead of compatible of handle the hardware configuration
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  .../devicetree/bindings/pwm/pwm-stm32.txt          | 38 ++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> new file mode 100644
>> index 0000000..575b9fb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> @@ -0,0 +1,38 @@
>> +STMicroelectronics PWM driver bindings for STM32
>
> Technically this bindings describe devices, so "driver binding" is a
> somewhat odd wording. Perhaps:
>
>         STMicroelectronics STM32 General Purpose Timer PWM bindings
>
> ?
 done

>
>> +
>> +Must be a sub-node of STM32 general purpose timer driver
>> +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
>
> Again, "driver parent node" is odd. Perhaps:
>
>         Must be a sub-node of an STM32 General Purpose Timer device tree
>         node. See ../mfd/stm32-general-purpose-timer.txt for details about
>         the parent node.
>
> ?

done

>
>> +Required parameters:
>> +- compatible:                Must be "st,stm32-pwm"
>> +- pinctrl-names:     Set to "default".
>> +- pinctrl-0:                 List of phandles pointing to pin configuration nodes
>> +                     for PWM module.
>> +                     For Pinctrl properties, please refer to [1].
>
> Your indentation and capitalization are inconsistent. Also, please refer
> to the pinctrl bindings by relative path and inline, rather than as a
> footnote reference.

OK

>
>> +
>> +Optional parameters:
>> +- st,breakinput:     Set if the hardware have break input capabilities
>> +- st,breakinput-polarity: Set break input polarity. Default is 0
>> +                      The value define the active polarity:
>> +                       - 0 (active LOW)
>> +                       - 1 (active HIGH)
>
> Could we fold these into a single property? If st,breakinput-polarity is
> not present it could simply mean that there is no break input, and if it
> is present you don't have to rely on a default.

I need to know if I have to activate breakinput feature and on which level
I will rewrite it like that:
Optional parameters:
- st,breakinput-polarity-high: Set if break input polarity is active
on high level.
- st,breakinput-polarity-high: Set if break input polarity is active
on low level.

>> +- st,pwm-num-chan:   Number of available PWM channels.  Default is 0.
>
> The pwm- prefix is rather redundant since the node is already named pwm.
> Why not simply st,channels? Or simply channels, since it's not really
> anything specific to this hardware.
>
> Come to think of it, might be worth having a discussion with our DT
> gurus about what their stance is on using the # as prefix for numbers
> (such as in #address-cells or #size-cells). This could be #channels to
> mark it more explicitly as representing a count.
>
>> +- st,32bits-counter: Set if the hardware have a 32 bits counter
>> +- st,complementary:  Set if the hardware have complementary output channels
>
> "hardware has" and also maybe mention explicitly that this is a boolean
> property. Otherwise people might be left wondering what it should be set
> to. Or maybe word this differently to imply that it's boolean:
>
>         - st,32bits-counter: if present, the hardware has a 32 bit counter
>         - st,complementary: if present, the hardware has a complementary
>                             output channel

I found a way to detect, at probe time, the number of channels, counter size,
break input capability and complementary channels so I will remove
"st,breakinput", "st,32bits-counter", "st,complementary" and "st,pwm-num-chan"
parameters

>
> Thierry

^ permalink raw reply

* [RFC PATCH 00/29] arm64: Scalable Vector Extension core support
From: Szabolcs Nagy @ 2016-12-05 11:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <0293f7d3-b3d3-1a68-5b99-75db195eb796@redhat.com>

On 05/12/16 10:44, Florian Weimer wrote:
>>> By the way, how is this implemented?  Some of them overlap existing
>>> callee-saved registers.
>>
>> Basically, all the *new* state is caller-save.
>>
>> The Neon/FPSIMD regs V8-V15 are callee-save, so in the SVE view
>> Zn[bits 127:0] is callee-save for all n = 8..15.
> 
> Are the extension parts of registers v8 to v15 used for argument passing?
> 
> If not, we should be able to use the existing dynamic linker trampoline.
> 

if sve arguments are passed to a function then it
has special call abi (which is probably not yet
documented), this call abi requires that such a
call does not go through plt to avoid requiring
sve aware libc.

same for tls access: the top part of sve regs have
to be saved by the caller before accessing tls so
the tlsdesc entry does not have to save them.

so current trampolines should be fine.

^ permalink raw reply

* [GIT PULL 2/2] DaVinci defconfig updates for v4.10 (part 4)
From: Sekhar Nori @ 2016-12-05 11:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205110316.18514-1-nsekhar@ti.com>

The following changes since commit 213971e7571e27f47b4e926904a9adf794925c51:

  ARM: davinci_all_defconfig: Enable OHCI as module (2016-11-22 20:50:46 +0530)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v4.10/defconfig-4

for you to fetch changes up to 47d03e48efa572109229aec9436948d92f44b689:

  ARM: davinci_all_defconfig: Enable da8xx usb otg (2016-12-01 19:52:12 +0530)

----------------------------------------------------------------
Enable support for MUSB based USB OTG on DA850.

----------------------------------------------------------------
Alexandre Bailon (1):
      ARM: davinci_all_defconfig: Enable da8xx usb otg

 arch/arm/configs/davinci_all_defconfig | 2 ++
 1 file changed, 2 insertions(+)

^ permalink raw reply

* [GIT PULL 1/2] DaVinci DT updates for v4.10 (part 4)
From: Sekhar Nori @ 2016-12-05 11:03 UTC (permalink / raw)
  To: linux-arm-kernel

The following changes since commit 878e908ad95637dc6567a9b5f6876a580ae90dfa:

  ARM: dts: da850: enable memctrl and mstpri nodes per board (2016-11-28 15:51:29 +0530)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/nsekhar/linux-davinci.git tags/davinci-for-v4.10/dt-4

for you to fetch changes up to e4ce904d6289f2a72affd2a0f0a44da6e5d0cce4:

  ARM: dts: da850: enable high speed for mmc (2016-12-05 14:20:44 +0530)

----------------------------------------------------------------
- Add device tree nodes for pin pull-up/pull-down
bias control on DA850.

- Enable high speed support on DA850 MMC/SD

----------------------------------------------------------------
Axel Haslam (1):
      ARM: dts: da850: enable high speed for mmc

David Lechner (1):
      ARM: dts: da850: Add node for pullup/pulldown pinconf

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

^ permalink raw reply

* [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm
From: Benjamin Gaignard @ 2016-12-05 11:02 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205072357.GB18069@ulmo.ba.sec>

2016-12-05 8:23 GMT+01:00 Thierry Reding <thierry.reding@gmail.com>:
> On Fri, Dec 02, 2016 at 11:17:19AM +0100, Benjamin Gaignard wrote:
>> This driver add support for pwm driver on stm32 platform.
>
> "adds". Also please use PWM in prose because it's an abbreviation.
>
>> The SoC have multiple instances of the hardware IP and each
>> of them could have small differences: number of channels,
>> complementary output, counter register size...
>> Use DT parameters to handle those differentes configuration
>
> "different configurations"

ok

>
>>
>> version 2:
>> - only keep one comptatible
>> - use DT paramaters to discover hardware block configuration
>
> "parameters"

ok

>
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  drivers/pwm/Kconfig     |   8 ++
>>  drivers/pwm/Makefile    |   1 +
>>  drivers/pwm/pwm-stm32.c | 285 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 294 insertions(+)
>>  create mode 100644 drivers/pwm/pwm-stm32.c
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index bf01288..a89fdba 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -388,6 +388,14 @@ config PWM_STI
>>         To compile this driver as a module, choose M here: the module
>>         will be called pwm-sti.
>>
>> +config PWM_STM32
>> +     bool "STMicroelectronics STM32 PWM"
>> +     depends on ARCH_STM32
>> +     depends on OF
>> +     select MFD_STM32_GP_TIMER
>
> Should that be a "depends on"?

I think select is fine because the wanted feature is PWM not the mfd part

>
>> +     help
>> +       Generic PWM framework driver for STM32 SoCs.
>> +
>>  config PWM_STMPE
>>       bool "STMPE expander PWM export"
>>       depends on MFD_STMPE
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index 1194c54..5aa9308 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -37,6 +37,7 @@ obj-$(CONFIG_PWM_ROCKCHIP)  += pwm-rockchip.o
>>  obj-$(CONFIG_PWM_SAMSUNG)    += pwm-samsung.o
>>  obj-$(CONFIG_PWM_SPEAR)              += pwm-spear.o
>>  obj-$(CONFIG_PWM_STI)                += pwm-sti.o
>> +obj-$(CONFIG_PWM_STM32)              += pwm-stm32.o
>>  obj-$(CONFIG_PWM_STMPE)              += pwm-stmpe.o
>>  obj-$(CONFIG_PWM_SUN4I)              += pwm-sun4i.o
>>  obj-$(CONFIG_PWM_TEGRA)              += pwm-tegra.o
>> diff --git a/drivers/pwm/pwm-stm32.c b/drivers/pwm/pwm-stm32.c
>> new file mode 100644
>> index 0000000..a362f63
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-stm32.c
>> @@ -0,0 +1,285 @@
>> +/*
>> + * Copyright (C) STMicroelectronics 2016
>> + * Author:  Gerald Baeza <gerald.baeza@st.com>
>
> Could use a blank line between the above. Also, please use a single
> space after : for consistency.

ok

>
>> + * License terms:  GNU General Public License (GPL), version 2
>
> Here too.

OK

>
>> + *
>> + * Inspired by timer-stm32.c from Maxime Coquelin
>> + *             pwm-atmel.c from Bo Shen
>> + */
>> +
>> +#include <linux/of.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pwm.h>
>
> Please sort these alphabetically.

ok

>> +
>> +#include <linux/mfd/stm32-gptimer.h>
>> +
>> +#define DRIVER_NAME "stm32-pwm"
>> +
>> +#define CAP_COMPLEMENTARY    BIT(0)
>> +#define CAP_32BITS_COUNTER   BIT(1)
>> +#define CAP_BREAKINPUT               BIT(2)
>> +#define CAP_BREAKINPUT_POLARITY BIT(3)
>
> Just make these boolean. Makes the conditionals a lot simpler to read.

I will do that for v4

>> +
>> +struct stm32_pwm_dev {
>
> No need for the _dev suffix.
>
>> +     struct device *dev;
>> +     struct clk *clk;
>> +     struct regmap *regmap;
>> +     struct pwm_chip chip;
>
> It's slightly more efficient to put this as first field because then
> to_stm32_pwm() becomes a no-op.

Ok I will remove it

>
>> +     int caps;
>> +     int npwm;
>
> unsigned int, please.
>
>> +     u32 polarity;
>
> Maybe use a prefix here to stress that it is the polarity of the
> complementary output. Otherwise one might take it for the PWM signal's
> polarity that's already part of the PWM state.

I will rename it

>
>> +};
>> +
>> +#define to_stm32_pwm_dev(x) container_of(chip, struct stm32_pwm_dev, chip)
>
> Please turn this into a static inline.

with putting struct pwm_chip as first filed I will just cast the structure

>> +
>> +static u32 __active_channels(struct stm32_pwm_dev *pwm_dev)
>
> No need for a __ prefix.

I wlll remove it

>
>> +{
>> +     u32 ccer;
>> +
>> +     regmap_read(pwm_dev->regmap, TIM_CCER, &ccer);
>> +
>> +     return ccer & TIM_CCER_CCXE;
>> +}
>> +
>> +static int write_ccrx(struct stm32_pwm_dev *dev, struct pwm_device *pwm,
>> +                   u32 ccr)
>
> u32 value, perhaps? I first mistook this to be a register offset.

OK

>
>> +{
>> +     switch (pwm->hwpwm) {
>> +     case 0:
>> +             return regmap_write(dev->regmap, TIM_CCR1, ccr);
>> +     case 1:
>> +             return regmap_write(dev->regmap, TIM_CCR2, ccr);
>> +     case 2:
>> +             return regmap_write(dev->regmap, TIM_CCR3, ccr);
>> +     case 3:
>> +             return regmap_write(dev->regmap, TIM_CCR4, ccr);
>> +     }
>> +     return -EINVAL;
>> +}
>> +
>> +static int stm32_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
>> +                         int duty_ns, int period_ns)
>
> Please implement this as an atomic PWM driver, I don't want new drivers
> to use the legacy callbacks.

Ok I will just use .apply ops.

>
>> +{
>> +     struct stm32_pwm_dev *dev = to_stm32_pwm_dev(chip);
>
> I think something like "stm", or "priv" would be more appropriate here.
> If you ever need access to a struct device, you'll be hard-pressed to
> find a good name for it.

I will use priv

>
>> +     unsigned long long prd, div, dty;
>> +     int prescaler = 0;
>
> If this can never be negative, please make it unsigned int.
>
>> +     u32 max_arr = 0xFFFF, ccmr, mask, shift, bdtr;
>> +
>> +     if (dev->caps & CAP_32BITS_COUNTER)
>> +             max_arr = 0xFFFFFFFF;
>
> I prefer lower-case hexadecimal digits.

I have found a way to remove this code

>
>> +
>> +     /* Period and prescaler values depends of clock rate */
>
> "depend on"
fixed

>
>> +     div = (unsigned long long)clk_get_rate(dev->clk) * period_ns;
>> +
>> +     do_div(div, NSEC_PER_SEC);
>> +     prd = div;
>> +
>> +     while (div > max_arr) {
>> +             prescaler++;
>> +             div = prd;
>> +             do_div(div, (prescaler + 1));
>> +     }
>> +     prd = div;
>
> Blank line after blocks, please.

fixed

>
>> +
>> +     if (prescaler > MAX_TIM_PSC) {
>> +             dev_err(chip->dev, "prescaler exceeds the maximum value\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     /* All channels share the same prescaler and counter so
>> +      * when two channels are active at the same we can't change them
>> +      */
>
> This isn't proper block comment style. Also, please use all of the
> columns at your disposal.

done

>
>> +     if (__active_channels(dev) & ~(1 << pwm->hwpwm * 4)) {
>> +             u32 psc, arr;
>> +
>> +             regmap_read(dev->regmap, TIM_PSC, &psc);
>> +             regmap_read(dev->regmap, TIM_ARR, &arr);
>> +
>> +             if ((psc != prescaler) || (arr != prd - 1))
>> +                     return -EINVAL;
>
> Maybe -EBUSY to differentiate from other error cases?

done

>
>> +     }
>> +
>> +     regmap_write(dev->regmap, TIM_PSC, prescaler);
>> +     regmap_write(dev->regmap, TIM_ARR, prd - 1);
>> +     regmap_update_bits(dev->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>> +
>> +     /* Calculate the duty cycles */
>> +     dty = prd * duty_ns;
>> +     do_div(dty, period_ns);
>> +
>> +     write_ccrx(dev, pwm, dty);
>> +
>> +     /* Configure output mode */
>> +     shift = (pwm->hwpwm & 0x1) * 8;
>> +     ccmr = (TIM_CCMR_PE | TIM_CCMR_M1) << shift;
>> +     mask = 0xFF << shift;
>> +
>> +     if (pwm->hwpwm & 0x2)
>
> This looks as though TIM_CCMR1 is used for channels 0 and 1, while
> TIM_CCMR2 is used for channels 2 and 3. Wouldn't it be more natural to
> make the conditional above:
>
>         if (pwm->hwpwm >= 2)
>
> ? Or perhaps better yet:
>
>         if (pwm->hwpwm < 2)
>                 /* update TIM_CCMR1 */
>         else
>                 /* update TIM_CCMR2 */

I will code it that, thanks.

>
> The other alternative, which might make the code slightly more readable,
> would be to get rid of all these conditionals by parameterizing the
> offsets per PWM channel. The PWM subsystem has a means of storing per-
> channel chip-specific data (see pwm_{set,get}_chip_data()). It might be
> a little overkill for this particular driver, given that the number of
> conditionals is fairly small.
>
>> +             regmap_update_bits(dev->regmap, TIM_CCMR2, mask, ccmr);
>> +     else
>> +             regmap_update_bits(dev->regmap, TIM_CCMR1, mask, ccmr);
>> +
>> +     if (!(dev->caps & CAP_BREAKINPUT))
>> +             return 0;
>
> If you make these capabilities boolean, it becomes much more readable,
> especially for negations:

Yes I will fix that

>
>         if (!dev->has_breakinput)
>
>> +
>> +     bdtr = TIM_BDTR_MOE | TIM_BDTR_AOE;
>> +
>> +     if (dev->caps & CAP_BREAKINPUT_POLARITY)
>> +             bdtr |= TIM_BDTR_BKE;
>> +
>> +     if (dev->polarity)
>> +             bdtr |= TIM_BDTR_BKP;
>> +
>> +     regmap_update_bits(dev->regmap, TIM_BDTR,
>> +                        TIM_BDTR_MOE | TIM_BDTR_AOE |
>> +                        TIM_BDTR_BKP | TIM_BDTR_BKE,
>> +                        bdtr);
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
>> +                               enum pwm_polarity polarity)
>> +{
>> +     u32 mask;
>> +     struct stm32_pwm_dev *dev = to_stm32_pwm_dev(chip);
>> +
>> +     mask = TIM_CCER_CC1P << (pwm->hwpwm * 4);
>> +     if (dev->caps & CAP_COMPLEMENTARY)
>> +             mask |= TIM_CCER_CC1NP << (pwm->hwpwm * 4);
>> +
>> +     regmap_update_bits(dev->regmap, TIM_CCER, mask,
>> +                        polarity == PWM_POLARITY_NORMAL ? 0 : mask);
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
>> +{
>> +     u32 mask;
>> +     struct stm32_pwm_dev *dev = to_stm32_pwm_dev(chip);
>> +
>> +     clk_enable(dev->clk);
>> +
>> +     /* Enable channel */
>> +     mask = TIM_CCER_CC1E << (pwm->hwpwm * 4);
>> +     if (dev->caps & CAP_COMPLEMENTARY)
>> +             mask |= TIM_CCER_CC1NE << (pwm->hwpwm * 4);
>> +
>> +     regmap_update_bits(dev->regmap, TIM_CCER, mask, mask);
>> +
>> +     /* Make sure that registers are updated */
>> +     regmap_update_bits(dev->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
>> +
>> +     /* Enable controller */
>> +     regmap_update_bits(dev->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
>> +
>> +     return 0;
>> +}
>> +
>> +static void stm32_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
>> +{
>> +     u32 mask;
>> +     struct stm32_pwm_dev *dev = to_stm32_pwm_dev(chip);
>> +
>> +     /* Disable channel */
>> +     mask = TIM_CCER_CC1E << (pwm->hwpwm * 4);
>> +     if (dev->caps & CAP_COMPLEMENTARY)
>> +             mask |= TIM_CCER_CC1NE << (pwm->hwpwm * 4);
>> +
>> +     regmap_update_bits(dev->regmap, TIM_CCER, mask, 0);
>> +
>> +     /* When all channels are disabled, we can disable the controller */
>> +     if (!__active_channels(dev))
>> +             regmap_update_bits(dev->regmap, TIM_CR1, TIM_CR1_CEN, 0);
>> +
>> +     clk_disable(dev->clk);
>> +}
>
> All of the above can be folded into the ->apply() hook for atomic PWM
> support.
>
> Also, in the above you use clk_enable() in the ->enable() callback and
> clk_disable() in ->disable(). If you need the clock to access registers
> you'll have to enabled it in the others as well because there are no
> guarantees that configuration will only happen between ->enable() and
> ->disable(). Atomic PWM simplifies this a bit, but you still need to be
> careful about when to enable the clock in your ->apply() hook.

I have used regmap functions enable/disable clk for me when accessing to
the registers.
I only have to take care of clk enable/disable when PWM state change.

>
>> +
>> +static const struct pwm_ops stm32pwm_ops = {
>> +     .config = stm32_pwm_config,
>> +     .set_polarity = stm32_pwm_set_polarity,
>> +     .enable = stm32_pwm_enable,
>> +     .disable = stm32_pwm_disable,
>> +};
>
> You need to set the .owner field as well.

OK and I will remove legacy ops to use only apply.

>
>> +
>> +static int stm32_pwm_probe(struct platform_device *pdev)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     struct device_node *np = dev->of_node;
>> +     struct stm32_gptimer_dev *mfd = dev_get_drvdata(pdev->dev.parent);
>> +     struct stm32_pwm_dev *pwm;
>> +     int ret;
>> +
>> +     pwm = devm_kzalloc(dev, sizeof(*pwm), GFP_KERNEL);
>> +     if (!pwm)
>> +             return -ENOMEM;
>> +
>> +     pwm->regmap = mfd->regmap;
>> +     pwm->clk = mfd->clk;
>> +
>> +     if (!pwm->regmap || !pwm->clk)
>> +             return -EINVAL;
>> +
>> +     if (of_property_read_bool(np, "st,complementary"))
>> +             pwm->caps |= CAP_COMPLEMENTARY;
>> +
>> +     if (of_property_read_bool(np, "st,32bits-counter"))
>> +             pwm->caps |= CAP_32BITS_COUNTER;
>> +
>> +     if (of_property_read_bool(np, "st,breakinput"))
>> +             pwm->caps |= CAP_BREAKINPUT;
>> +
>> +     if (!of_property_read_u32(np, "st,breakinput-polarity", &pwm->polarity))
>> +             pwm->caps |= CAP_BREAKINPUT_POLARITY;
>> +
>> +     of_property_read_u32(np, "st,pwm-num-chan", &pwm->npwm);
>> +
>> +     pwm->chip.base = -1;
>> +     pwm->chip.dev = dev;
>> +     pwm->chip.ops = &stm32pwm_ops;
>> +     pwm->chip.npwm = pwm->npwm;
>> +
>> +     ret = pwmchip_add(&pwm->chip);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     platform_set_drvdata(pdev, pwm);
>> +
>> +     return 0;
>> +}
>> +
>> +static int stm32_pwm_remove(struct platform_device *pdev)
>> +{
>> +     struct stm32_pwm_dev *pwm = platform_get_drvdata(pdev);
>> +     int i;
>
> unsigned int, please.
OK

>
>> +
>> +     for (i = 0; i < pwm->npwm; i++)
>> +             pwm_disable(&pwm->chip.pwms[i]);
>> +
>> +     pwmchip_remove(&pwm->chip);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct of_device_id stm32_pwm_of_match[] = {
>> +     {
>> +             .compatible = "st,stm32-pwm",
>> +     },
>
> The above can be collapsed into a single line.

OK

>
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_pwm_of_match);
>> +
>> +static struct platform_driver stm32_pwm_driver = {
>> +     .probe          = stm32_pwm_probe,
>> +     .remove         = stm32_pwm_remove,
>> +     .driver = {
>> +             .name   = DRIVER_NAME,
>> +             .of_match_table = stm32_pwm_of_match,
>> +     },
>> +};
>
> Please don't use tabs for padding within the structure definition since
> it doesn't align properly anyway.

OK

>
>> +module_platform_driver(stm32_pwm_driver);
>> +
>> +MODULE_ALIAS("platform:" DRIVER_NAME);
>> +MODULE_DESCRIPTION("STMicroelectronics STM32 PWM driver");
>> +MODULE_LICENSE("GPL");
>
> According to the header comment this should be "GPL v2".

done

>
> Thanks,
> Thierry

^ permalink raw reply

* [PATCH v2 2/3] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Icenowy Zheng @ 2016-12-05 11:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205094023.mtymfvpmca4x3ohh@lukather>



05.12.2016, 17:40, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Mon, Dec 05, 2016 at 04:59:44PM +0800, Icenowy Zheng wrote:
>> ?2016?12?5? 16:52? Maxime Ripard <maxime.ripard@free-electrons.com>???
>> ?>
>> ?> On Fri, Dec 02, 2016 at 10:22:30PM +0800, Icenowy Zheng wrote:
>> ?> >
>> ?> >
>> ?> > 01.12.2016, 17:36, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
>> ?> > > On Mon, Nov 28, 2016 at 12:29:07AM +0000, Andr? Przywara wrote:
>> ?> > >> ?> Something more interesting happened.
>> ?> > >> ?>
>> ?> > >> ?> Xunlong made a add-on board for Orange Pi Zero, which exposes the
>> ?> > >> ?> two USB Controllers exported at expansion bus as USB Type-A
>> ?> > >> ?> connectors.
>> ?> > >> ?>
>> ?> > >> ?> Also it exposes a analog A/V jack and a microphone.
>> ?> > >> ?>
>> ?> > >> ?> Should I enable {e,o}hci{2.3} in the device tree?
>> ?> > >>
>> ?> > >> ?Actually we should do this regardless of this extension board. The USB
>> ?> > >> ?pins are not multiplexed and are exposed on user accessible pins (just
>> ?> > >> ?not soldered, but that's a detail), so I think they qualify for DT
>> ?> > >> ?enablement. And even if a user can't use them, it doesn't hurt to have
>> ?> > >> ?them (since they are not multiplexed).
>> ?> > >
>> ?> > > My main concern about this is that we'll leave regulators enabled by
>> ?> > > default, for a minority of users. And that minority will prevent to do
>> ?> > > a proper power management when the times come since we'll have to keep
>> ?> > > that behaviour forever.
>> ?> >
>> ?> > I think these users can add a 'fdt set /xxx/xxx status "disabled" ' .
>> ?>
>> ?> You can't ask that from the majority of users. These users will take
>> ?> debian or fedora, install it, and expect everything to work
>> ?> properly. I would make the opposite argument actually. If someone is
>> ?> knowledgeable enough to solder the USB pins a connector, then (s)he'll
>> ?> be able to make that u-boot call.
>>
>> ?Now (s)he do not need soldering.
>>
>> ?(S)he needs only paying $1.99 more to Xunlong to get the expansion
>> ?board, and insert it on the OPi Zero.
>
> Which is going to require an overlay anyway, so we could have the USB
> bits in there too.

If so, I think the [PATCH -next v3 2/2] is ready to be merged ;-)

>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com

^ permalink raw reply

* [PATCH 2/2] arm64: alternatives: Work around NOP generation with broken assembler
From: Marc Zyngier @ 2016-12-05 10:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205100545.GA14058@arm.com>

On 05/12/16 10:05, Will Deacon wrote:
> On Sat, Dec 03, 2016 at 02:05:38PM +0000, Marc Zyngier wrote:
>> When compiling a .inst directive in an alternative clause with
>> a rather old version of gas (circa 2.24), and when used with
>> the alternative_else_nop_endif feature, the compilation fails
>> with a rather cryptic message such as:
>>
>> arch/arm64/lib/clear_user.S:33: Error: bad or irreducible absolute expression
>>
>> which is caused by the bug described in eb7c11ee3c5c ("arm64:
>> alternative: Work around .inst assembler bugs").
>>
>> This effectively prevents the use of the "nops" macro, which
>> requires the number of instruction as a parameter (the assembler
>> is confused and unable to compute the difference between two labels).
>>
>> As an alternative(!), use the .fill directive to output the number
>> of required NOPs (.fill has the good idea to output the fill pattern
>> in the endianness of the instruction stream).
> 
> Are you sure about that? The gas docs say:
> 
> `The contents of each repeat bytes is taken from an 8-byte number. The
>  highest order 4 bytes are zero. The lowest order 4 bytes are value rendered
>  in the byte-order of an integer on the computer as is assembling for.'
> 
> and I'd expect "integer" to refer to data values, rather than instructions.

My tests on 2.24 and 2.25 seem to show that the output is always LE,
which could be another GAS issue. I've asked the binutils people for
information.

> 
>>
>> Fixes: 792d47379f4d ("arm64: alternative: add auto-nop infrastructure")
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>>  arch/arm64/include/asm/alternative.h | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
>> index 39feb85..39f8c98 100644
>> --- a/arch/arm64/include/asm/alternative.h
>> +++ b/arch/arm64/include/asm/alternative.h
>> @@ -159,9 +159,19 @@ void apply_alternatives(void *start, size_t length);
>>   * of NOPs. The number of NOPs is chosen automatically to match the
>>   * previous case.
>>   */
>> +
>> +#define NOP_INST	0xd503201f
> 
> If we define this, let's put it in insn.h or something. It could even be
> used in the vdso linker script, which open codes this constant.

Sure.

> 
>> +
>>  .macro alternative_else_nop_endif
>>  alternative_else
>> -	nops	(662b-661b) / AARCH64_INSN_SIZE
>> +	/*
>> +	 * The same assembler bug strikes again here if the first half
>> +	 * of the alternative sequence contains a .inst, leading to a
>> +	 * bizarre error message. Fortulately, .fill replaces the
> 
> Fortunately
> 
>> +	 * "nops" macro by inserting padding with the target machine
>> +	 * endianness.
>> +	 */
>> +	.fill	(662b-661b) / AARCH64_INSN_SIZE, AARCH64_INSN_SIZE, NOP_INST
> 
> Assuming we can get .fill to do the right thing, we should probably use
> it in __nops rather than open-coding it here. Or is the issue that we're
> unable to pass (662b-661b) / AARCH64_INSN_SIZE to any macro?

Having an intermediate macro seems to work (probably because this is not
actually evaluated until it reaches .fill). I'll update the patch if I
get confirmation of the validity of the workaround from the binutils people.

Otherwise, we'll have to find another alternative, and maybe go back to
not using the auto-nop for SET_PSTATE_PAN/UAO.

Thanks,

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

^ permalink raw reply

* [PATCH v9 07/16] drivers: acpi: implement acpi_dma_configure
From: Lorenzo Pieralisi @ 2016-12-05 10:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <003001d24edd$40d7d030$c2877090$@codeaurora.org>

On Mon, Dec 05, 2016 at 03:22:02PM +0530, Sricharan wrote:
> Hi Lorenzo,
> 
> >
> >On Sat, Dec 3, 2016 at 11:39 AM, Lorenzo Pieralisi
> ><lorenzo.pieralisi@arm.com> wrote:
> >> On Sat, Dec 03, 2016 at 03:11:09AM +0100, Rafael J. Wysocki wrote:
> >>> On Fri, Dec 2, 2016 at 4:38 PM, Lorenzo Pieralisi
> >>> <lorenzo.pieralisi@arm.com> wrote:
> >>> > Rafael, Mark, Suravee,
> >>> >
> >>> > On Mon, Nov 21, 2016 at 10:01:39AM +0000, Lorenzo Pieralisi wrote:
> >>> >> On DT based systems, the of_dma_configure() API implements DMA
> >>> >> configuration for a given device. On ACPI systems an API equivalent to
> >>> >> of_dma_configure() is missing which implies that it is currently not
> >>> >> possible to set-up DMA operations for devices through the ACPI generic
> >>> >> kernel layer.
> >>> >>
> >>> >> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> >>> >> calls that for now are just wrappers around arch_setup_dma_ops() and
> >>> >> arch_teardown_dma_ops() and also updates ACPI and PCI core code to use
> >>> >> the newly introduced acpi_dma_configure/acpi_dma_deconfigure functions.
> >>> >>
> >>> >> Since acpi_dma_configure() is used to configure DMA operations, the
> >>> >> function initializes the dma/coherent_dma masks to sane default values
> >>> >> if the current masks are uninitialized (also to keep the default values
> >>> >> consistent with DT systems) to make sure the device has a complete
> >>> >> default DMA set-up.
> >>> >
> >>> > I spotted a niggle that unfortunately was hard to spot (and should not
> >>> > be a problem per se but better safe than sorry) and I am not comfortable
> >>> > with it.
> >>> >
> >>> > Following commit d0562674838c ("ACPI / scan: Parse _CCA and setup
> >>> > device coherency") in acpi_bind_one() we check if the acpi_device
> >>> > associated with a device just added supports DMA, first it was
> >>> > done with acpi_check_dma() and then commit 1831eff876bd ("device
> >>> > property: ACPI: Make use of the new DMA Attribute APIs") changed
> >>> > it to acpi_get_dma_attr().
> >>> >
> >>> > The subsequent check (attr != DEV_DMA_NOT_SUPPORTED) is always true
> >>> > on _any_ acpi device we pass to acpi_bind_one() on x86, which was
> >>> > fine because we used it to call arch_setup_dma_ops(), which is a nop
> >>> > on x86. On ARM64 a _CCA method is required to define if a device
> >>> > supports DMA so (attr != DEV_DMA_NOT_SUPPORTED) may well be false.
> >>> >
> >>> > Now, acpi_bind_one() is used to bind an acpi_device to its physical
> >>> > node also for pseudo-devices like cpus and memory nodes. For those
> >>> > objects, on x86, attr will always be != DEV_DMA_NOT_SUPPORTED.
> >>> >
> >>> > So far so good, because on x86 arch_setup_dma_ops() is empty code.
> >>> >
> >>> > With this patch, I use the (attr != DEV_DMA_NOT_SUPPORTED) check
> >>> > to call acpi_dma_configure() which is basically a nop on x86 except
> >>> > that it sets up the dma_mask/coherent_dma_mask to a sane default value
> >>> > (after all we are setting up DMA for the device so it makes sense to
> >>> > initialize the masks there if they were unset since we are configuring
> >>> > DMA for the device in question) for the given device.
> >>> >
> >>> > Problem is, as per the explanation above, we are also setting the
> >>> > default dma masks for pseudo-devices (eg CPUs) that were previously
> >>> > untouched, it should not be a problem per-se but I am not comfortable
> >>> > with that, honestly it does not make much sense.
> >>> >
> >>> > An easy "fix" would be to move the default dma masks initialization out
> >>> > of acpi_dma_configure() (as it was in previous patch versions of this
> >>> > series - I moved it to acpi_dma_configure() just a consolidation point
> >>> > for initializing the masks instead of scattering them in every
> >>> > acpi_dma_configure caller) I can send this as a fix-up patch to Joerg if
> >>> > we think that's the right thing to do (or I can send it to Rafael later
> >>> > when the code is in the merged depending on the timing) just let me
> >>> > know please.
> >>>
> >>> Why can't arch_setup_dma_ops() set those masks too?
> >>
> >> Because the dma masks set-up is done by the caller (see
> >> of_dma_configure()) according to firmware configuration or
> >> platform data knowledge. I wanted to replicate the of_dma_configure()
> >> interface on ACPI for obvious reasons (on ARM systems), I stopped
> >> short of adding ACPI code to mirror of_dma_get_range() equivalent
> >> (through the _DMA object) but I am really really nervous about changing
> >> the code path on x86 because in theory all is fine, in practice even
> >> just setting the masks to sane values can have unexpected consequences,
> >> I just can't know (that's why I wasn't doing it in the first iterations
> >> of this series).
> >>
> >> Side note: DT with of_dma_configure() and ACPI with
> >> acpi_create_platform_device() set the default dma mask for all
> >> platform devices already _regardless_ of what they really are, though
> >> arguably acpi_bind_one() touches ways more devices.
> >>
> >> I really think that removing the default dma masks settings from
> >> acpi_dma_configure() is the safer thing to do for the time being (or
> >> moving acpi_dma_configure() to acpi_create_platform_device(), where the
> >> DMA masks are set-up by default by core ACPI. Mark, Suravee, what was
> >> the rationale behind calling arch_setup_dma_ops() in acpi_bind_one() ?)
> >
> >Alternatively, you can add one more arch wrapper that will be a no-op
> >on x86 and that will set up the default masks and call
> >arch_setup_dma_ops() on ARM.  Then, you can invoke that from
> >acpi_dma_configure().
> >
> >Or make the definition of acpi_dma_configure() itself depend on the
> >architecture.
> >
> 
> So is it better that either removing the masks from acpi_dma_configure
> (or) creating the wrapper as Rafael mentioned, than moving
> acpi_dma_configure itself , because with something like iommu probe
> deferral that is tried, acpi_dma_configure is getting invoked from a
> device's really_probe, a different path again ?

Yes, I thought about that too. Given what I said above (ie I would like
to extend the mask set-up with _DMA object - that is generic ACPI but
can affect legacy x86 - and if that does not work through IORT specific
bindings), as per Rafael suggestion I added an iort_set_dma_mask wrapper
that is a NOP on x86, leaving acpi_dma_configure() unchanged for ARM64.

Patch coming, thanks everyone.

Lorenzo

^ permalink raw reply

* [PATCH] i2c: rk3x: keep i2c irq ON in suspend
From: Heiko Stuebner @ 2016-12-05 10:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480924979-13450-1-git-send-email-david.wu@rock-chips.com>

Hi David,

Am Montag, 5. Dezember 2016, 16:02:59 CET schrieb David Wu:
> During suspend there may still be some i2c access happening.
> And if we don't keep i2c irq ON, there may be i2c access timeout if
> i2c is in irq mode of operation.

can you describe the issue you're trying to fix a bit more please?

I.e. I'd think the i2c-core does suspend i2c-client devices first, so that
these should be able to finish up their ongoing transfers and not start any
new ones instead?

Your irq can still happen slightly after the system started going to actually
sleep, so to me it looks like you just widened the window where irqs can be
handled. Especially as your irq could also just simply stem from the start
state, so you cannot even be sure if your transaction actually is finished.

So to me it looks like the i2c-connected device driver should be fixed instead?

In the past I solved this for example in the zforce_ts driver [0] to
prevent i2c transfers from happening during suspend.


Heiko

[0] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/zforce_ts.c


> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> ---
>  drivers/i2c/busses/i2c-rk3x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index df22066..67af32a 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -1261,7 +1261,7 @@ static int rk3x_i2c_probe(struct platform_device
> *pdev) }
> 
>  	ret = devm_request_irq(&pdev->dev, irq, rk3x_i2c_irq,
> -			       0, dev_name(&pdev->dev), i2c);
> +			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), i2c);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "cannot request IRQ\n");
>  		return ret;

^ permalink raw reply

* next-20161205 build: 3 failures 4 warnings (next-20161205)
From: Marc Zyngier @ 2016-12-05 10:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205104455.2m5ky3hgd5n2kb2w@sirena.org.uk>

On 05/12/16 10:44, Mark Brown wrote:
> On Mon, Dec 05, 2016 at 07:56:06AM +0000, Build bot for Mark Brown wrote:
> 
> Today's -next fails to build an arm64 allmodconfig:  
> 
>> 	arm64-allmodconfig
>> ../arch/arm64/include/asm/probes.h:18:25: fatal error: asm/opcodes.h: No such file or directory
> 
> due to bca8f17f57bd76d (arm64: Get rid of asm/opcodes.h) having missed
> one reference to the header.

Fix on the list: https://www.spinics.net/lists/arm-kernel/msg546960.html

Thanks,

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

^ permalink raw reply

* [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings
From: Thierry Reding @ 2016-12-05 10:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161205083535.GB14004@dell>

On Mon, Dec 05, 2016 at 08:35:35AM +0000, Lee Jones wrote:
> On Mon, 05 Dec 2016, Thierry Reding wrote:
> 
> > On Fri, Dec 02, 2016 at 11:17:18AM +0100, Benjamin Gaignard wrote:
> > > Define bindings for pwm-stm32
> > > 
> > > version 2:
> > > - use parameters instead of compatible of handle the hardware configuration
> > > 
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > ---
> > >  .../devicetree/bindings/pwm/pwm-stm32.txt          | 38 ++++++++++++++++++++++
> > >  1 file changed, 38 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> > > new file mode 100644
> > > index 0000000..575b9fb
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> > > @@ -0,0 +1,38 @@
> > > +STMicroelectronics PWM driver bindings for STM32
> > 
> > Technically this bindings describe devices, so "driver binding" is a
> > somewhat odd wording. Perhaps:
> > 
> > 	STMicroelectronics STM32 General Purpose Timer PWM bindings
> > 
> > ?
> > 
> > > +
> > > +Must be a sub-node of STM32 general purpose timer driver
> > > +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
> > 
> > Again, "driver parent node" is odd. Perhaps:
> > 
> > 	Must be a sub-node of an STM32 General Purpose Timer device tree
> > 	node. See ../mfd/stm32-general-purpose-timer.txt for details about
> > 	the parent node.
> > 
> > ?
> > 
> > > +Required parameters:
> > > +- compatible:		Must be "st,stm32-pwm"
> > > +- pinctrl-names: 	Set to "default".
> > > +- pinctrl-0: 		List of phandles pointing to pin configuration nodes
> > > +			for PWM module.
> > > +			For Pinctrl properties, please refer to [1].
> > 
> > Your indentation and capitalization are inconsistent. Also, please refer
> > to the pinctrl bindings by relative path and inline, rather than as a
> > footnote reference.
> > 
> > > +
> > > +Optional parameters:
> > > +- st,breakinput:	Set if the hardware have break input capabilities
> > > +- st,breakinput-polarity: Set break input polarity. Default is 0
> > > +			 The value define the active polarity:
> > > +			  - 0 (active LOW)
> > > +			  - 1 (active HIGH)
> > 
> > Could we fold these into a single property? If st,breakinput-polarity is
> > not present it could simply mean that there is no break input, and if it
> > is present you don't have to rely on a default.
> > 
> > > +- st,pwm-num-chan:	Number of available PWM channels.  Default is 0.
> > 
> > The pwm- prefix is rather redundant since the node is already named pwm.
> > Why not simply st,channels? Or simply channels, since it's not really
> > anything specific to this hardware.
> > 
> > Come to think of it, might be worth having a discussion with our DT
> > gurus about what their stance is on using the # as prefix for numbers
> > (such as in #address-cells or #size-cells). This could be #channels to
> > mark it more explicitly as representing a count.
> 
> Unfortunately that ship has sailed.
> 
> st,pwm-num-chan already exists (with your blessing).  It's usually

I think I did at the time object, though very mildly. The property here
is somewhat different, though. For one this is a PWM specific node, so
the pwm- prefix is completely redundant. Also for pwm-sti where you had
introduced st,pwm-num-chan, the property denoted how many PWM channels
vs. capture channels (st,capture-num-chan) the device was supposed to
use. Here there are only one type of channels.

> suggested to reuse exiting properties when writing new bindings.

Given the above I think this case is different. Further my understanding
is that the desire to reuse existing properties is primarily for generic
properties. Vendor specific properties are always going to have to be
defined in the specific bindings, so it doesn't matter very much whether
they are reused or not.

Lastly, I think st,pwm-num-chan is not optimal, and while it isn't very
bad either, I do believe that when we see ways of improving things then
we should do so, regardless of whether existing ways to describe things
already exist. Especially if it comes at no additional cost.

All of that said, this is my opinion and if everybody thinks that the
st,pwm-num-chan is the better choice I'll merge it. Anyway, we'll need
the Acked-by from one of the device tree bindings maintainers and I'd
like to see at least an attempt at a discussion.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161205/36f76394/attachment.sig>

^ permalink raw reply

* [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: Corentin Labbe @ 2016-12-05 10:48 UTC (permalink / raw)
  To: linux-arm-kernel

From: LABBE Corentin <clabbe.montjoie@gmail.com>

The Security System have a PRNG.
This patch add support for it as an hwrng.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
Changes since v1:
 - Replaced all spin_lock_bh by simple spin_lock
 - Removed handling of size not modulo 4 which will never happen
 - Added add_random_ready_callback()

 drivers/crypto/Kconfig                   |  8 +++
 drivers/crypto/sunxi-ss/Makefile         |  1 +
 drivers/crypto/sunxi-ss/sun4i-ss-core.c  | 14 +++++
 drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c | 99 ++++++++++++++++++++++++++++++++
 drivers/crypto/sunxi-ss/sun4i-ss.h       |  9 +++
 5 files changed, 131 insertions(+)
 create mode 100644 drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 4d2b81f..38f7aca 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -538,6 +538,14 @@ config CRYPTO_DEV_SUN4I_SS
 	  To compile this driver as a module, choose M here: the module
 	  will be called sun4i-ss.
 
+config CRYPTO_DEV_SUN4I_SS_PRNG
+	bool "Support for Allwinner Security System PRNG"
+	depends on CRYPTO_DEV_SUN4I_SS
+	select HW_RANDOM
+	help
+	  This driver provides kernel-side support for the Pseudo-Random
+	  Number Generator found in the Security System.
+
 config CRYPTO_DEV_ROCKCHIP
 	tristate "Rockchip's Cryptographic Engine driver"
 	depends on OF && ARCH_ROCKCHIP
diff --git a/drivers/crypto/sunxi-ss/Makefile b/drivers/crypto/sunxi-ss/Makefile
index 8f4c7a2..ca049ee 100644
--- a/drivers/crypto/sunxi-ss/Makefile
+++ b/drivers/crypto/sunxi-ss/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sun4i-ss.o
 sun4i-ss-y += sun4i-ss-core.o sun4i-ss-hash.o sun4i-ss-cipher.o
+sun4i-ss-$(CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG) += sun4i-ss-hwrng.o
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index 3ac6c6c..fa739de 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -359,6 +359,16 @@ static int sun4i_ss_probe(struct platform_device *pdev)
 		}
 	}
 	platform_set_drvdata(pdev, ss);
+
+#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG
+	/* Voluntarily made the PRNG optional */
+	err = sun4i_ss_hwrng_register(&ss->hwrng);
+	if (!err)
+		dev_info(ss->dev, "sun4i-ss PRNG loaded");
+	else
+		dev_err(ss->dev, "sun4i-ss PRNG failed");
+#endif
+
 	return 0;
 error_alg:
 	i--;
@@ -386,6 +396,10 @@ static int sun4i_ss_remove(struct platform_device *pdev)
 	int i;
 	struct sun4i_ss_ctx *ss = platform_get_drvdata(pdev);
 
+#ifdef CONFIG_CRYPTO_DEV_SUN4I_SS_PRNG
+	sun4i_ss_hwrng_remove(&ss->hwrng);
+#endif
+
 	for (i = 0; i < ARRAY_SIZE(ss_algs); i++) {
 		switch (ss_algs[i].type) {
 		case CRYPTO_ALG_TYPE_ABLKCIPHER:
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c b/drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c
new file mode 100644
index 0000000..8319cae
--- /dev/null
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-hwrng.c
@@ -0,0 +1,99 @@
+#include "sun4i-ss.h"
+
+static void sun4i_ss_seed(struct random_ready_callback *rdy)
+{
+	struct sun4i_ss_ctx *ss;
+
+	ss = container_of(rdy, struct sun4i_ss_ctx, random_ready);
+	get_random_bytes(ss->seed, SS_SEED_LEN);
+	ss->random_ready.func = NULL;
+}
+
+static int sun4i_ss_hwrng_init(struct hwrng *hwrng)
+{
+	struct sun4i_ss_ctx *ss;
+	int ret;
+
+	ss = container_of(hwrng, struct sun4i_ss_ctx, hwrng);
+
+	ss->random_ready.owner = THIS_MODULE;
+	ss->random_ready.func = sun4i_ss_seed;
+
+	ret = add_random_ready_callback(&ss->random_ready);
+	switch (ret) {
+	case 0:
+		break;
+	case -EALREADY:
+		get_random_bytes(ss->seed, SS_SEED_LEN);
+		ss->random_ready.func = NULL;
+		ret = 0;
+		break;
+	default:
+		ss->random_ready.func = NULL;
+	}
+
+	return ret;
+}
+
+static int sun4i_ss_hwrng_read(struct hwrng *hwrng, void *buf,
+			       size_t max, bool wait)
+{
+	int i;
+	u32 v;
+	u32 *data = buf;
+	const u32 mode = SS_OP_PRNG | SS_PRNG_CONTINUE | SS_ENABLED;
+	size_t len;
+	struct sun4i_ss_ctx *ss;
+
+	ss = container_of(hwrng, struct sun4i_ss_ctx, hwrng);
+	len = min_t(size_t, SS_DATA_LEN, max);
+
+	/* If the PRNG is not seeded */
+	if (ss->random_ready.func)
+		return -EAGAIN;
+
+	spin_lock(&ss->slock);
+
+	writel(mode, ss->base + SS_CTL);
+
+	/* write the seed */
+	for (i = 0; i < SS_SEED_LEN / 4; i++)
+		writel(ss->seed[i], ss->base + SS_KEY0 + i * 4);
+	writel(mode | SS_PRNG_START, ss->base + SS_CTL);
+
+	/* Read the random data */
+	readsl(ss->base + SS_TXFIFO, data, len / 4);
+
+	/* Update the seed */
+	for (i = 0; i < SS_SEED_LEN / 4; i++) {
+		v = readl(ss->base + SS_KEY0 + i * 4);
+		ss->seed[i] = v;
+	}
+
+	writel(0, ss->base + SS_CTL);
+	spin_unlock(&ss->slock);
+	return len;
+}
+
+int sun4i_ss_hwrng_register(struct hwrng *hwrng)
+{
+	hwrng->name = "sun4i Security System PRNG";
+	hwrng->init = sun4i_ss_hwrng_init;
+	hwrng->read = sun4i_ss_hwrng_read;
+	hwrng->quality = 1000;
+
+	/* Cannot use devm_hwrng_register() since we need to hwrng_unregister
+	 * before stopping clocks/regulator
+	 */
+	return hwrng_register(hwrng);
+}
+
+void sun4i_ss_hwrng_remove(struct hwrng *hwrng)
+{
+	struct sun4i_ss_ctx *ss;
+
+	ss = container_of(hwrng, struct sun4i_ss_ctx, hwrng);
+	if (ss->random_ready.func)
+		del_random_ready_callback(&ss->random_ready);
+	hwrng_unregister(hwrng);
+}
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss.h b/drivers/crypto/sunxi-ss/sun4i-ss.h
index f04c0f8..85772d7 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss.h
+++ b/drivers/crypto/sunxi-ss/sun4i-ss.h
@@ -23,6 +23,7 @@
 #include <linux/scatterlist.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
+#include <linux/hw_random.h>
 #include <crypto/md5.h>
 #include <crypto/sha.h>
 #include <crypto/hash.h>
@@ -125,6 +126,9 @@
 #define SS_RXFIFO_EMP_INT_ENABLE	(1 << 2)
 #define SS_TXFIFO_AVA_INT_ENABLE	(1 << 0)
 
+#define SS_SEED_LEN (192 / 8)
+#define SS_DATA_LEN (160 / 8)
+
 struct sun4i_ss_ctx {
 	void __iomem *base;
 	int irq;
@@ -134,6 +138,9 @@ struct sun4i_ss_ctx {
 	struct device *dev;
 	struct resource *res;
 	spinlock_t slock; /* control the use of the device */
+	struct random_ready_callback random_ready;
+	struct hwrng hwrng;
+	u32 seed[SS_SEED_LEN / 4];
 };
 
 struct sun4i_ss_alg_template {
@@ -199,3 +206,5 @@ int sun4i_ss_des_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
 			unsigned int keylen);
 int sun4i_ss_des3_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
 			 unsigned int keylen);
+int sun4i_ss_hwrng_register(struct hwrng *hwrng);
+void sun4i_ss_hwrng_remove(struct hwrng *hwrng);
-- 
2.7.3

^ permalink raw reply related

* next-20161205 build: 3 failures 4 warnings (next-20161205)
From: Mark Brown @ 2016-12-05 10:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <E1cDo86-0008IX-9W@optimist>

On Mon, Dec 05, 2016 at 07:56:06AM +0000, Build bot for Mark Brown wrote:

Today's -next fails to build an arm64 allmodconfig:  

> 	arm64-allmodconfig
> ../arch/arm64/include/asm/probes.h:18:25: fatal error: asm/opcodes.h: No such file or directory

due to bca8f17f57bd76d (arm64: Get rid of asm/opcodes.h) having missed
one reference to the header.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161205/8b5dd2e2/attachment-0001.sig>

^ permalink raw reply

* [RFC PATCH 00/29] arm64: Scalable Vector Extension core support
From: Florian Weimer @ 2016-12-05 10:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161201103048.GO1574@e103592.cambridge.arm.com>

On 12/01/2016 11:30 AM, Dave Martin wrote:

>> The VL value is implicitly thread-local data, and the encoded state may have
>> an implicit dependency on it, although it does not contain vector registers
>> as such.
>
> This doesn't sound like an absolute requirement to me.
>
> If we presume that the SVE registers never need to get saved or
> restored, what stops the context data format being VL-independent?

I'm concerned the suspended computation has code which has been selected 
to fit a particular VL value.

 > If the save/restore logic doesn't touch SVE, which would its
 > implementation be VL-dependent?

Because it has been optimized for a certain vector length?

>>> Because the SVE procedure call standard determines that the SVE
>>> registers are caller-save,
>>
>> By the way, how is this implemented?  Some of them overlap existing
>> callee-saved registers.
>
> Basically, all the *new* state is caller-save.
>
> The Neon/FPSIMD regs V8-V15 are callee-save, so in the SVE view
> Zn[bits 127:0] is callee-save for all n = 8..15.

Are the extension parts of registers v8 to v15 used for argument passing?

If not, we should be able to use the existing dynamic linker trampoline.

Thanks,
Florian

^ permalink raw reply

* [PATCH v3 3/3] ARM: da850: fix da850_set_pll0rate()
From: Sekhar Nori @ 2016-12-05 10:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480932549-30811-4-git-send-email-bgolaszewski@baylibre.com>

On Monday 05 December 2016 03:39 PM, Bartosz Golaszewski wrote:
> This function is confusing - its second argument is an index to the
> freq table, not the requested clock rate in Hz, but it's used as the
> set_rate callback for the pll0 clock. It leads to an oops when the
> caller doesn't know the internals and passes the rate in Hz as
> argument instead of the cpufreq index since this argument isn't bounds
> checked either.
> 
> Fix it by iterating over the array of supported frequencies and
> selecting a one that matches or returning -EINVAL for unsupported
> rates.
> 
> Also: update the davinci cpufreq driver. It's the only user of this
> clock and currently it passes the cpufreq table index to
> clk_set_rate(), which is confusing. Make it pass the requested clock
> rate in Hz.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  arch/arm/mach-davinci/da850.c     | 20 ++++++++++++++++----
>  drivers/cpufreq/davinci-cpufreq.c |  2 +-
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 006ec56..9837541 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -1179,14 +1179,26 @@ static int da850_set_armrate(struct clk *clk, unsigned long index)
>  	return clk_set_rate(pllclk, index);
>  }
>  
> -static int da850_set_pll0rate(struct clk *clk, unsigned long index)
> +static int da850_set_pll0rate(struct clk *clk, unsigned long rate)
>  {
> -	unsigned int prediv, mult, postdiv;
> -	struct da850_opp *opp;
>  	struct pll_data *pll = clk->pll_data;
> +	struct cpufreq_frequency_table *freq;
> +	unsigned int prediv, mult, postdiv;
> +	struct da850_opp *opp = NULL;
>  	int ret;
>  
> -	opp = (struct da850_opp *) cpufreq_info.freq_table[index].driver_data;
> +	for (freq = da850_freq_table;
> +	     freq->frequency != CPUFREQ_TABLE_END; freq++) {
> +		/* rate is in Hz, freq->frequency is in KHz */
> +		if (freq->frequency == rate / 1000) {

Or
	rate = rate / 1000;

before the loop begins? The idea behind my comment was mostly to reduce
the amount of calculation in the loop.

Thanks,
Sekhar

^ permalink raw reply

* [PATCH v3 1/3] ARM: da850: fix infinite loop in clk_set_rate()
From: Sekhar Nori @ 2016-12-05 10:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAMpxmJUYksWf-p3KMYZ_oxKWELit6MwAWd7i__6sQY0-_nKOyA@mail.gmail.com>

On Monday 05 December 2016 04:02 PM, Bartosz Golaszewski wrote:
> 2016-12-05 11:15 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>> On Monday 05 December 2016 03:39 PM, Bartosz Golaszewski wrote:
>>> The aemif clock is added twice to the lookup table in da850.c. This
>>> breaks the children list of pll0_sysclk3 as we're using the same list
>>> links in struct clk. When calling clk_set_rate(), we get stuck in
>>> propagate_rate().
>>>
>>> Create a separate clock for nand, inheriting the rate of the aemif
>>> clock and retrieve it in the davinci_nand module.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> ---
>>>  arch/arm/mach-davinci/da850.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>>> index e770c97..c008e5e 100644
>>> --- a/arch/arm/mach-davinci/da850.c
>>> +++ b/arch/arm/mach-davinci/da850.c
>>> @@ -367,6 +367,11 @@ static struct clk aemif_clk = {
>>>       .flags          = ALWAYS_ENABLED,
>>>  };
>>>
>>> +static struct clk aemif_nand_clk = {
>>> +     .name           = "nand",
>>> +     .parent         = &aemif_clk,
>>> +};
>>> +
>>>  static struct clk usb11_clk = {
>>>       .name           = "usb11",
>>>       .parent         = &pll0_sysclk4,
>>> @@ -537,7 +542,7 @@ static struct clk_lookup da850_clks[] = {
>>>       CLK("da830-mmc.0",      NULL,           &mmcsd0_clk),
>>>       CLK("da830-mmc.1",      NULL,           &mmcsd1_clk),
>>>       CLK("ti-aemif",         NULL,           &aemif_clk),
>>> -     CLK(NULL,               "aemif",        &aemif_clk),
>>> +     CLK(NULL,               "aemif",        &aemif_nand_clk),
>>
>> Why use a NULL device name here? Same question was asked on v2
> 
> Eek, sorry, I missed that.

For next version, can you also add a comment on top of 'struct clk
aemif_nand_clk' explaining why its needed?

Thanks,
Sekhar

^ permalink raw reply

* [PATCH v3 1/3] ARM: da850: fix infinite loop in clk_set_rate()
From: Bartosz Golaszewski @ 2016-12-05 10:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <689efaa1-d8d9-6937-5880-3ed7a1401268@ti.com>

2016-12-05 11:15 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> On Monday 05 December 2016 03:39 PM, Bartosz Golaszewski wrote:
>> The aemif clock is added twice to the lookup table in da850.c. This
>> breaks the children list of pll0_sysclk3 as we're using the same list
>> links in struct clk. When calling clk_set_rate(), we get stuck in
>> propagate_rate().
>>
>> Create a separate clock for nand, inheriting the rate of the aemif
>> clock and retrieve it in the davinci_nand module.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  arch/arm/mach-davinci/da850.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> index e770c97..c008e5e 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
>> @@ -367,6 +367,11 @@ static struct clk aemif_clk = {
>>       .flags          = ALWAYS_ENABLED,
>>  };
>>
>> +static struct clk aemif_nand_clk = {
>> +     .name           = "nand",
>> +     .parent         = &aemif_clk,
>> +};
>> +
>>  static struct clk usb11_clk = {
>>       .name           = "usb11",
>>       .parent         = &pll0_sysclk4,
>> @@ -537,7 +542,7 @@ static struct clk_lookup da850_clks[] = {
>>       CLK("da830-mmc.0",      NULL,           &mmcsd0_clk),
>>       CLK("da830-mmc.1",      NULL,           &mmcsd1_clk),
>>       CLK("ti-aemif",         NULL,           &aemif_clk),
>> -     CLK(NULL,               "aemif",        &aemif_clk),
>> +     CLK(NULL,               "aemif",        &aemif_nand_clk),
>
> Why use a NULL device name here? Same question was asked on v2

Eek, sorry, I missed that.

> submission. Also, can you please make sure you are testing this in both
> DT mode (da850-lcdk) and non-DT boot (da850-evm).

Will do.

Thanks,
Bartosz

^ permalink raw reply

* [PATCH 06/11] ARM: dts: imx: Add imx6sll EVK board dts support
From: Fabio Estevam @ 2016-12-05 10:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480660774-25055-7-git-send-email-ping.bai@nxp.com>

On Fri, Dec 2, 2016 at 4:39 AM, Bai Ping <ping.bai@nxp.com> wrote:
> Add basic dts support for imx6sll EVK baoard.

s/baord/board

> +       battery: max8903 at 0 {
> +               compatible = "fsl,max8903-charger";
> +               pinctrl-names = "default";
> +               dok_input = <&gpio4 13 1>;
> +               uok_input = <&gpio4 13 1>;
> +               chg_input = <&gpio4 15 1>;
> +               flt_input = <&gpio4 14 1>;
> +               fsl,dcm_always_high;
> +               fsl,dc_valid;
> +               fsl,adc_disable;

These properties do not exist in mainline, please remove them.


> +               status = "okay";
> +       };
> +
> +       pxp_v4l2_out {
> +               compatible = "fsl,imx6sl-pxp-v4l2";
> +               status = "okay";
> +       };

We don't have pxp support in mainline kernel, please remove it.

> +
> +       regulators {
> +               compatible = "simple-bus";
> +               #address-cells = <1>;
> +               #size-cells = <0>;


Please remove it and place the regulator nodes directly as below:


> +
> +               reg_usb_otg1_vbus: regulator at 0 {
> +                       compatible = "regulator-fixed";
> +                       reg = <0>;
> +                       regulator-name = "usb_otg1_vbus";
> +                       regulator-min-microvolt = <5000000>;
> +                       regulator-max-microvolt = <5000000>;
> +                       gpio = <&gpio4 0 GPIO_ACTIVE_HIGH>;
> +                       enable-active-high;
> +               };

    reg_usb_otg1_vbus: regulator-usb-otg1-vbus {
          compatible = "regulator-fixed";
          regulator-name = "usb_otg1_vbus";
          regulator-min-microvolt = <5000000>;
          regulator-max-microvolt = <5000000>;
          gpio = <&gpio4 0 GPIO_ACTIVE_HIGH>;
          enable-active-high;
    };

> +&cpu0 {
> +       arm-supply = <&sw1a_reg>;
> +       soc-supply = <&sw1c_reg>;
> +};

This is only for LDO bypass mode, right? We don't support LDO-bypass
in mainline.

> +&gpc {
> +       fsl,ldo-bypass = <1>;

We don't support ldo-bypass in mainline, please remove it.

> +&pxp {
> +       status = "okay";
> +};

We don't support PXP in mainline, please remove it.

^ permalink raw reply

* [RFC PATCH] PCI: designware: add host_init() error handling
From: Joao Pinto @ 2016-12-05 10:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <d416f9b3-20d9-0339-a4d2-9a4ba0669633@linaro.org>

?s 11:51 AM de 12/2/2016, Srinivas Kandagatla escreveu:
> 
> 
> On 02/12/16 10:32, Joao Pinto wrote:
>>
>> Hi Srinivas,
>>
>> ?s 11:51 AM de 12/1/2016, Srinivas Kandagatla escreveu:
>>>  drivers/pci/host/pci-dra7xx.c           |  4 +++-
>>>  drivers/pci/host/pci-exynos.c           |  4 +++-
>>>  drivers/pci/host/pci-imx6.c             |  4 +++-
>>>  drivers/pci/host/pci-keystone.c         |  4 +++-
>>>  drivers/pci/host/pci-layerscape.c       | 12 ++++++++----
>>>  drivers/pci/host/pcie-armada8k.c        |  4 +++-
>>>  drivers/pci/host/pcie-designware-plat.c |  4 +++-
>>>  drivers/pci/host/pcie-designware.c      |  4 +++-
>>>  drivers/pci/host/pcie-designware.h      |  2 +-
>>>  drivers/pci/host/pcie-qcom.c            |  6 ++++--
>>>  drivers/pci/host/pcie-spear13xx.c       |  4 +++-
>>>  11 files changed, 37 insertions(+), 15 deletions(-)
>>>
>>
>> Thanks for the patch!
>>
>> In my opinion your idea is good but only qcom driver is able to detect failure
>> in the specific host init routine, all others have a 'return 0' even if
>> something not well init. I would recomend that we take this issue a bit further
>> and add the error checking to all specific pci drivers in order to make them as
>> robust as qcom'.
> I totally agree with you, I can give this a go in next version.

Sure, but I think it would be better to finish now since we are on top of the
task. I can help you if you need.

Thanks Joao

> 
> Thanks,
> srini
> 
>>
>> Thanks,
>> Joao
>>

^ 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