Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2016-12-28 22:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161228212139.adkixdgvmtj2ukjs@pengutronix.de>

Hello Uwe,

2016-12-28 22:21 GMT+01:00 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> Hello Cedric,
>
> On Fri, Dec 23, 2016 at 01:41:15PM +0100, M'boumba Cedric Madianga wrote:
>> > I don't understand why DUTY is required to reach 400 kHz. Given a parent
>> > freq of 30 MHz, with CCR = 25 and DUTY = 0 we have:
>> >
>> >         t_high = 25 * 33.333 ns = 833.333 ns
>> >         t_low = 2 * 25 * 33.333 ns = 1666.667 ns
>> >
>> > then t_high and t_low satisfy the i2c bus specification
>> > (t_low > 1300 ns, t_high > 600 ns) and we have t_low + t_high = 2500 ns
>> > = 1 / 400 kHz.
>> >
>> > Where is the error?
>> Hum ok you are right. I was a bad interpretation of the datasheet.
>> So now it is clearer. Thanks for that.
>> I will correct and improve my comments in the V8.
>
> The benefit of DUTY=1 is (I think) that you can reach 400 kHz also with
> lower parent frequencies. And so it't probably sensible to make use of
> it unconditionally for fast mode.
Ok I agree.

>
>> >> + * So, in order to cover both SCL high/low with Duty = 1,
>> >> + * CCR = 16 * SCL period * I2C CLK frequency
>> >
>> > I don't get that. Actually you need to use low + high, so
>> > CCR = parentrate / (25 * 400 kHz), right?
>> With your new inputs above, I think I could use a simpler implementation:
>> CCR = scl_high_period * parent_rate
>> with scl_high_period = 5 ?s in standard mode to reach 100khz
>> and scl_high_period = 1 ?s in fast mode to reach 400khz with 1/2 or
>> 16/9 duty cycle.
>> So, I am wondering if I have to let the customer setting the duty
>> cycle in the DT for example with "st,duty=0" or "st,duty=1" property
>> (0 for 1/2 and 1 for 16/9).
>> Or perhaps the best option it to use a default value. What is your
>> feeling regarding this point ?
>
> No, don't add a property to the device tree. Just take pencil and paper
> and think a while about the right algorithm to choose the right register
> settings.
Ok thanks

>
>
>> >> +     cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> >> +     freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
>> >> +
>> >> +     if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
>> >> +             trise = freq + 1;
>> >> +     else
>> >> +             trise = freq * 300 / 1000 + 1;
>> >
>> > if freq is big such that freq * 300 overflows does this result in a
>> > wrong result, or does the compiler optimize correctly?
>> For sure the compiler will never exceeds u32 max value
>
> If I compile
> -------->8--------
> #include <stdio.h>
>
> void myfunc(unsigned freq)
> {
>         unsigned trise = freq * 300 / 1000 + 1;
>         printf("trise = %u", trise);
> }
>
> -------->8--------
>
> for arm with -O3 I get:
>
> 00000000 <myfunc>:
>    0:   e3a01f4b        mov     r1, #300        ; 0x12c
>    4:   e0010190        mul     r1, r0, r1
>    8:   e59f3010        ldr     r3, [pc, #16]   ; 20 <myfunc+0x20>
>    c:   e59f0010        ldr     r0, [pc, #16]   ; 24 <myfunc+0x24>
>   10:   e0812193        umull   r2, r1, r3, r1
>   14:   e1a01321        lsr     r1, r1, #6
>   18:   e2811001        add     r1, r1, #1
>   1c:   eafffffe        b       0 <printf>
>   20:   10624dd3        .word   0x10624dd3
>   24:   00000000        .word   0x00000000
>
> The mul instruction at offset 4 writes the least significant 32 bits of
> 300 * r0 to r1 and so doesn't handle overflow correctly.
In case of overflow, the parent frequency has to be at least at 1431657 Mhz.
The STM32F4 SoC will never reach this value for any parent clock.
So, I think that this point is not really critical and you can
probably keep these simple lines of code without adding u32 overflow
checking for big frequency.

>
>> > This is still not really understandable.
>> I have already added some comments from datasheet to explain the
>> different cases.
>> I don't see how I could be more understandable as it is clearly the
>> hardware way of working...
>
> You need to comment the check for the length, something like:
>
>         /*
>          * To end the transfer correctly the foo bit has to be cleared
>          * already on the last but one byte to be transferred.
>          */
>
OK I will add more comments.

> and bonus points if you can shrink the number of functions that check
> for this stuff.
There are only 2 functions to handle this stuff:  handle_read() for
RXNE event and handle_rx_btf() for BTF event
I would prefer to keep 2 seperate functions to handle each event
according to buffer length as I don't have to do the same thing.
For example:
- for RXNE event with count = 2, I have to disable buffer interrupts
- for BTF event with msg count = 2, I have to .generate stop or
repeated start and disable all remaining interrupts.
I think that if I gather this stuff in one function, the code will be
less understandable.

>
>> > Just do:
>> >
>> >         if (status & STM32F4_I2C_SR1_SB) {
>> >                 ...
>> >         }
>> >
>> >         if (status & ...) {
>> >
>> >         }
>> ok but I would prefer something like that:
>> flag = status & possible_status
>> if (flag & STM32F4_I2C_SR1_SB) {
>> ...
>> }
>>
>> if (flag & ...) {
>> }
>
> Ok, if you really need possible_status.
>
>> > Then it's obvious by reading the code in which order they are handled
>> > without the need to check the definitions. Do you really need to jugle
>> > with possible_status?
>> I think I have to use possible_status as some events could occur
>> whereas the corresponding interrupt is disabled.
>> For example, for a 2 byte-reception, we don't have to take into accout
>> RXNE event so the corresponding interrupt is disabled.
>
> Is it possible to make it more obvious by doing:
>
>         status = read_from_status_register() & read_from_interrupt_enable_register();
>
> at a single place?
Ok I will add these 2 functions in order to only use one status variable.

>
>> >> +     /* Use __fls() to check error bits first */
>> >> +     flag = __fls(status & possible_status);
>> >> +
>> >> +     switch (1 << flag) {
>> >> +     case STM32F4_I2C_SR1_BERR:
>> >> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
>> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
>> >> +             msg->result = -EIO;
>> >> +             break;
>> >> +
>> >> +     case STM32F4_I2C_SR1_ARLO:
>> >> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
>> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
>> >> +             msg->result = -EAGAIN;
>> >> +             break;
>> >> +
>> >> +     case STM32F4_I2C_SR1_AF:
>> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> >> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> >> +             msg->result = -EIO;
>> >> +             break;
>> >> +
>> >> +     default:
>> >> +             dev_err(i2c_dev->dev,
>> >> +                     "err it unhandled: status=0x%08x)\n", status);
>> >> +             return IRQ_NONE;
>> >> +     }
>> >
>> > You only check a single irq flag here.
>> Yes only the first error could be reported to the i2c clients via
>> msg->result that's why I don't check all errors.
>> Moreover, as soon as an error occurs, the I2C device is reset.
>
> The the "first" in the comment for __fls is misleading. Also do:
>
>         if (status & MOST_IMPORTANT_ERROR) {
>                 ...
>         }
>
>         if (status & SECOND_MOST_IMPORTANT_ERROR) {
>                 ...
>         }
>
> (if you need including possible_status stuff).
OK

>
> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-K?nig            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Best regards,
Cedric

^ permalink raw reply

* [PATCH v5 0/2] qcom OTG regulator support
From: Stephen Boyd @ 2016-12-28 22:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161018004251.28733-1-stephen.boyd@linaro.org>

Hi Sebastian,

On Mon, Oct 17, 2016 at 5:42 PM, Stephen Boyd <stephen.boyd@linaro.org> wrote:
> This is a resend/resurrection of Tim's patches to add support
> for the OTG regulator on some of qcom's PMICs[1]. I've made
> some minor modifications to the driver to make it work, but
> otherwise it works fine with my USB otg testing. Changes
> are noted in a maintainer tag.
>
> Tim, did you want to me to fix the name? I pulled these from patchwork
> and it seems you sent it as "Bird, Tim" which may have not been
> intentional.
>

Can this be picked up for v4.11? Tim hasn't responded so I assume it's
fine with the name.

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Uwe Kleine-König @ 2016-12-28 21:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOMZO5BD3Y6vXwt7_bdv_KyGvKSYru1vzDowsD81fzEbM4gHPA@mail.gmail.com>

On Wed, Dec 28, 2016 at 07:33:26PM -0200, Fabio Estevam wrote:
> On Wed, Dec 28, 2016 at 6:54 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > I'd do it the other way around and so get more and simpler patches:
> >
> >  - make driver aware of i.MX35
> >  - fix dtsi to use i.MX35 type where applicable
> 
> This would not work for old dtb's as they are not aware of the  i.mx35 type.

Right, so they would fall back to the i.MX21 type and not profit from
Markus' change until either the dtb is updated or the third commit is
added. I'll try to contact Markus to ask if he remembers more details.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH] ARM: dts: r8a7794: link DU to VSPD
From: Sergei Shtylyov @ 2016-12-28 21:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529351.Fac5N2tKoF@wasted.cogentembedded.com>

Add the "vsps" property to the DU device node in order to link this node to
the (single) VSPD node.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against the 'renesas-devel-20161220-v4.9' of Simon Horman's
'renesas.git' repo.  It's  only meaningful if the "Enable R8A7794 DU VSPD
compositor" DU driver patches are applied...

 arch/arm/boot/dts/r8a7794.dtsi |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: renesas/arch/arm/boot/dts/r8a7794.dtsi
===================================================================
--- renesas.orig/arch/arm/boot/dts/r8a7794.dtsi
+++ renesas/arch/arm/boot/dts/r8a7794.dtsi
@@ -908,7 +908,7 @@
 		power-domains = <&sysc R8A7794_PD_ALWAYS_ON>;
 	};
 
-	vsp1 at fe930000 {
+	vspd0: vsp1 at fe930000 {
 		compatible = "renesas,vsp1";
 		reg = <0 0xfe930000 0 0x8000>;
 		interrupts = <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>;
@@ -925,6 +925,7 @@
 		clocks = <&mstp7_clks R8A7794_CLK_DU0>,
 			 <&mstp7_clks R8A7794_CLK_DU0>;
 		clock-names = "du.0", "du.1";
+		vsps = <&vspd0>;
 		status = "disabled";
 
 		ports {

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Fabio Estevam @ 2016-12-28 21:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161228205419.la74jr4rfes7fa4v@pengutronix.de>

On Wed, Dec 28, 2016 at 6:54 PM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:

> I'd do it the other way around and so get more and simpler patches:
>
>  - make driver aware of i.MX35
>  - fix dtsi to use i.MX35 type where applicable

This would not work for old dtb's as they are not aware of the  i.mx35 type.

^ permalink raw reply

* [PATCH 2/4] ARM: dts: Add am335x-boneblack-wireless
From: Jason Kridner @ 2016-12-28 21:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161227175837.28970-2-robertcnelson@gmail.com>



> On Dec 27, 2016, at 11:58 AM, Robert Nelson <robertcnelson@gmail.com> wrote:
> 
> BeagleBone Black Wireless is clone of the BeagleBone Black (BBB) with the Ethernet
> replaced by a TI wl1835 wireless module.
> 
> This board can be indentified by the BWAx value after A335BNLT (BBB) in the at24 eeprom:
> BWAx [aa 55 33 ee 41 33 33 35  42 4e 4c 54 42 57 41 35  |.U3.A335BNLTBWA5|]

I believe the correct statement is BWxx, but BWBx reserves the option to be software incompatible in some way. My preference is to have it boot anyway, but I believe that is only dependent in the bootloader. 

> 
> http://beagleboard.org/black-wireless
> https://github.com/beagleboard/beaglebone-black-wireless
> 
> firmware: https://github.com/beagleboard/beaglebone-black-wireless/tree/master/firmware
> wl18xx mac address: /proc/device-tree/ocp/ethernet at 4a100000/slave at 4a100200/mac-address
> 
> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
> CC: Tony Lindgren <tony@atomide.com>
> CC: Jason Kridner <jkridner@beagleboard.org>
> ---
> arch/arm/boot/dts/Makefile                      |   1 +
> arch/arm/boot/dts/am335x-boneblack-wireless.dts | 109 ++++++++++++++++++++++++
> 2 files changed, 110 insertions(+)
> create mode 100644 arch/arm/boot/dts/am335x-boneblack-wireless.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index cccdbcb557b6..9415a49bd11b 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -563,6 +563,7 @@ dtb-$(CONFIG_SOC_AM33XX) += \
>    am335x-base0033.dtb \
>    am335x-bone.dtb \
>    am335x-boneblack.dtb \
> +    am335x-boneblack-wireless.dtb \
>    am335x-bonegreen.dtb \
>    am335x-chiliboard.dtb \
>    am335x-cm-t335.dtb \
> diff --git a/arch/arm/boot/dts/am335x-boneblack-wireless.dts b/arch/arm/boot/dts/am335x-boneblack-wireless.dts
> new file mode 100644
> index 000000000000..105bd10655f7
> --- /dev/null
> +++ b/arch/arm/boot/dts/am335x-boneblack-wireless.dts
> @@ -0,0 +1,109 @@
> +/*
> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +/dts-v1/;
> +
> +#include "am33xx.dtsi"
> +#include "am335x-bone-common.dtsi"
> +#include "am335x-boneblack-common.dtsi"
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> +    model = "TI AM335x BeagleBone Black Wireless";
> +    compatible = "ti,am335x-bone-black-wireless", "ti,am335x-bone-black", "ti,am335x-bone", "ti,am33xx";
> +
> +    wlan_en_reg: fixedregulator at 2 {
> +        compatible = "regulator-fixed";
> +        regulator-name = "wlan-en-regulator";
> +        regulator-min-microvolt = <1800000>;
> +        regulator-max-microvolt = <1800000>;
> +        startup-delay-us= <70000>;
> +
> +        /* WL_EN */
> +        gpio = <&gpio3 9 0>;
> +        enable-active-high;
> +    };
> +};
> +
> +&am33xx_pinmux {
> +    bt_pins: pinmux_bt_pins {
> +        pinctrl-single,pins = <
> +            AM33XX_IOPAD(0x928, PIN_OUTPUT_PULLUP | MUX_MODE7)    /* gmii1_txd0.gpio0_28 - BT_EN */
> +        >;
> +    };
> +
> +    mmc3_pins: pinmux_mmc3_pins {
> +        pinctrl-single,pins = <
> +            AM33XX_IOPAD(0x93c, PIN_INPUT_PULLUP | MUX_MODE6 ) /* (L15) gmii1_rxd1.mmc2_clk */
> +            AM33XX_IOPAD(0x914, PIN_INPUT_PULLUP | MUX_MODE6 ) /* (J16) gmii1_txen.mmc2_cmd */
> +            AM33XX_IOPAD(0x918, PIN_INPUT_PULLUP | MUX_MODE5 ) /* (J17) gmii1_rxdv.mmc2_dat0 */
> +            AM33XX_IOPAD(0x91c, PIN_INPUT_PULLUP | MUX_MODE5 ) /* (J18) gmii1_txd3.mmc2_dat1 */
> +            AM33XX_IOPAD(0x920, PIN_INPUT_PULLUP | MUX_MODE5 ) /* (K15) gmii1_txd2.mmc2_dat2 */
> +            AM33XX_IOPAD(0x908, PIN_INPUT_PULLUP | MUX_MODE5 ) /* (H16) gmii1_col.mmc2_dat3 */
> +        >;
> +    };
> +
> +    uart3_pins: pinmux_uart3_pins {
> +        pinctrl-single,pins = <
> +            AM33XX_IOPAD(0x934, PIN_INPUT_PULLUP | MUX_MODE1)    /* gmii1_rxd3.uart3_rxd */
> +            AM33XX_IOPAD(0x938, PIN_OUTPUT_PULLDOWN | MUX_MODE1)    /* gmii1_rxd2.uart3_txd */
> +            AM33XX_IOPAD(0x948, PIN_INPUT | MUX_MODE3)        /* mdio_data.uart3_ctsn */
> +            AM33XX_IOPAD(0x94c, PIN_OUTPUT_PULLDOWN | MUX_MODE3)    /* mdio_clk.uart3_rtsn */
> +        >;
> +    };
> +
> +    wl18xx_pins: pinmux_wl18xx_pins {
> +        pinctrl-single,pins = <
> +            AM33XX_IOPAD(0x92c, PIN_OUTPUT_PULLDOWN | MUX_MODE7)    /* gmii1_txclk.gpio3_9 WL_EN */
> +            AM33XX_IOPAD(0x944, PIN_INPUT_PULLDOWN | MUX_MODE7)    /* rmii1_refclk.gpio0_29 WL_IRQ */
> +            AM33XX_IOPAD(0x930, PIN_OUTPUT_PULLUP | MUX_MODE7)    /* gmii1_rxclk.gpio3_10 LS_BUF_EN */
> +        >;
> +    };
> +};
> +
> +&mac {
> +    status = "disabled";
> +};
> +
> +&mmc3 {
> +    dmas = <&edma_xbar 12 0 1
> +        &edma_xbar 13 0 2>;
> +    dma-names = "tx", "rx";
> +    status = "okay";
> +    vmmc-supply = <&wlan_en_reg>;
> +    bus-width = <4>;
> +    non-removable;
> +    cap-power-off-card;
> +    ti,needs-special-hs-handling;
> +    keep-power-in-suspend;
> +    pinctrl-names = "default";
> +    pinctrl-0 = <&mmc3_pins &wl18xx_pins>;
> +
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +    wlcore: wlcore at 2 {
> +        compatible = "ti,wl1835";
> +        reg = <2>;
> +        interrupt-parent = <&gpio0>;
> +        interrupts = <29 IRQ_TYPE_EDGE_RISING>;
> +    };
> +};
> +
> +&uart3 {
> +    pinctrl-names = "default";
> +    pinctrl-0 = <&uart3_pins &bt_pins>;
> +    status = "okay";
> +};
> +
> +&gpio3 {
> +    ls_buf_en {
> +        gpio-hog;
> +        gpios = <10 GPIO_ACTIVE_HIGH>;
> +        output-high;
> +        line-name = "LS_BUF_EN";
> +    };
> +};
> -- 
> 2.11.0
> 

^ permalink raw reply

* [PATCH v7 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2016-12-28 21:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOAejn3sc1F8T_rbSTr6-wBNncNwObeK+c6=_Q0BMahDdHXxQA@mail.gmail.com>

Hello Cedric,

On Fri, Dec 23, 2016 at 01:41:15PM +0100, M'boumba Cedric Madianga wrote:
> > I don't understand why DUTY is required to reach 400 kHz. Given a parent
> > freq of 30 MHz, with CCR = 25 and DUTY = 0 we have:
> >
> >         t_high = 25 * 33.333 ns = 833.333 ns
> >         t_low = 2 * 25 * 33.333 ns = 1666.667 ns
> >
> > then t_high and t_low satisfy the i2c bus specification
> > (t_low > 1300 ns, t_high > 600 ns) and we have t_low + t_high = 2500 ns
> > = 1 / 400 kHz.
> >
> > Where is the error?
> Hum ok you are right. I was a bad interpretation of the datasheet.
> So now it is clearer. Thanks for that.
> I will correct and improve my comments in the V8.

The benefit of DUTY=1 is (I think) that you can reach 400 kHz also with
lower parent frequencies. And so it't probably sensible to make use of
it unconditionally for fast mode.

> >> + * So, in order to cover both SCL high/low with Duty = 1,
> >> + * CCR = 16 * SCL period * I2C CLK frequency
> >
> > I don't get that. Actually you need to use low + high, so
> > CCR = parentrate / (25 * 400 kHz), right?
> With your new inputs above, I think I could use a simpler implementation:
> CCR = scl_high_period * parent_rate
> with scl_high_period = 5 ?s in standard mode to reach 100khz
> and scl_high_period = 1 ?s in fast mode to reach 400khz with 1/2 or
> 16/9 duty cycle.
> So, I am wondering if I have to let the customer setting the duty
> cycle in the DT for example with "st,duty=0" or "st,duty=1" property
> (0 for 1/2 and 1 for 16/9).
> Or perhaps the best option it to use a default value. What is your
> feeling regarding this point ?

No, don't add a property to the device tree. Just take pencil and paper
and think a while about the right algorithm to choose the right register
settings.


> >> +     cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> >> +     freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
> >> +
> >> +     if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD)
> >> +             trise = freq + 1;
> >> +     else
> >> +             trise = freq * 300 / 1000 + 1;
> >
> > if freq is big such that freq * 300 overflows does this result in a
> > wrong result, or does the compiler optimize correctly?
> For sure the compiler will never exceeds u32 max value

If I compile
-------->8--------
#include <stdio.h>

void myfunc(unsigned freq)
{
	unsigned trise = freq * 300 / 1000 + 1;
	printf("trise = %u", trise);
}

-------->8--------

for arm with -O3 I get:

00000000 <myfunc>:
   0:	e3a01f4b 	mov	r1, #300	; 0x12c
   4:	e0010190 	mul	r1, r0, r1
   8:	e59f3010 	ldr	r3, [pc, #16]	; 20 <myfunc+0x20>
   c:	e59f0010 	ldr	r0, [pc, #16]	; 24 <myfunc+0x24>
  10:	e0812193 	umull	r2, r1, r3, r1
  14:	e1a01321 	lsr	r1, r1, #6
  18:	e2811001 	add	r1, r1, #1
  1c:	eafffffe 	b	0 <printf>
  20:	10624dd3 	.word	0x10624dd3
  24:	00000000 	.word	0x00000000

The mul instruction at offset 4 writes the least significant 32 bits of
300 * r0 to r1 and so doesn't handle overflow correctly.

> > This is still not really understandable.
> I have already added some comments from datasheet to explain the
> different cases.
> I don't see how I could be more understandable as it is clearly the
> hardware way of working...

You need to comment the check for the length, something like:

	/*
	 * To end the transfer correctly the foo bit has to be cleared
	 * already on the last but one byte to be transferred.
	 */

and bonus points if you can shrink the number of functions that check
for this stuff.

> > Just do:
> >
> >         if (status & STM32F4_I2C_SR1_SB) {
> >                 ...
> >         }
> >
> >         if (status & ...) {
> >
> >         }
> ok but I would prefer something like that:
> flag = status & possible_status
> if (flag & STM32F4_I2C_SR1_SB) {
> ...
> }
> 
> if (flag & ...) {
> }

Ok, if you really need possible_status.

> > Then it's obvious by reading the code in which order they are handled
> > without the need to check the definitions. Do you really need to jugle
> > with possible_status?
> I think I have to use possible_status as some events could occur
> whereas the corresponding interrupt is disabled.
> For example, for a 2 byte-reception, we don't have to take into accout
> RXNE event so the corresponding interrupt is disabled.

Is it possible to make it more obvious by doing:

	status = read_from_status_register() & read_from_interrupt_enable_register();

at a single place?

> >> +     /* Use __fls() to check error bits first */
> >> +     flag = __fls(status & possible_status);
> >> +
> >> +     switch (1 << flag) {
> >> +     case STM32F4_I2C_SR1_BERR:
> >> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
> >> +             msg->result = -EIO;
> >> +             break;
> >> +
> >> +     case STM32F4_I2C_SR1_ARLO:
> >> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
> >> +             msg->result = -EAGAIN;
> >> +             break;
> >> +
> >> +     case STM32F4_I2C_SR1_AF:
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> >> +             msg->result = -EIO;
> >> +             break;
> >> +
> >> +     default:
> >> +             dev_err(i2c_dev->dev,
> >> +                     "err it unhandled: status=0x%08x)\n", status);
> >> +             return IRQ_NONE;
> >> +     }
> >
> > You only check a single irq flag here.
> Yes only the first error could be reported to the i2c clients via
> msg->result that's why I don't check all errors.
> Moreover, as soon as an error occurs, the I2C device is reset.

The the "first" in the comment for __fls is misleading. Also do:

	if (status & MOST_IMPORTANT_ERROR) {
		...
	}

	if (status & SECOND_MOST_IMPORTANT_ERROR) {
		...
	}

(if you need including possible_status stuff).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCHv2 net-next 00/16] net: mvpp2: add basic support for PPv2.2
From: Thomas Petazzoni @ 2016-12-28 21:08 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161228.120644.1166014191192724301.davem@davemloft.net>

Hello,

On Wed, 28 Dec 2016 12:06:44 -0500 (EST), David Miller wrote:

> > This series depends on the series named "net: mvpp2: misc improvements
> > and preparation patches".  
> 
> Please in the future only submit one patch series at a time.
> 
> If I've told you that a large patch series is hard to review and that
> therefore one should keep each submitted series small and to a
> reasonable size, that is completely undermined when you submit
> multiple series to work around that request.

Sure. I'll wait for the first patch series to be merged (potentially
after several iterations) before resending the second patch series.

Thanks for the feedback!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [GIT PULL] Qualcomm ARM64 Fixes for 4.10-rc1
From: Andy Gross @ 2016-12-28 20:58 UTC (permalink / raw)
  To: linux-arm-kernel

The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77:

  Linux 4.10-rc1 (2016-12-25 16:13:08 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git tags/qcom-fixes-for-4.10-rc1

for you to fetch changes up to e9112936f240856c925321fd2bd91adc7c00399c:

  arm64: dts: msm8996: Add required memory carveouts (2016-12-28 14:50:33 -0600)

----------------------------------------------------------------
Qualcomm ARM64 Fixes for v4.10-rc1

* Fix instability in MSM8996 due to incorrect carveouts

----------------------------------------------------------------
Stephen Boyd (1):
      arm64: dts: msm8996: Add required memory carveouts

 arch/arm64/boot/dts/qcom/msm8996.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Uwe Kleine-König @ 2016-12-28 20:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <c22600d5-f6b5-4c13-59b4-ba391ea29a4c@mleia.com>

Hello Vladimir,

On Sat, Dec 24, 2016 at 07:08:15AM +0200, Vladimir Zapolskiy wrote:
> Hello Uwe,
> 
> On 12/23/2016 04:11 PM, Uwe Kleine-K?nig wrote:
> > On Fri, Dec 23, 2016 at 01:21:20PM +0200, Vladimir Zapolskiy wrote:
> >> Hi Uwe,
> >>
> >> On 12/23/2016 12:01 PM, Uwe Kleine-K?nig wrote:
> >>> On Fri, Dec 23, 2016 at 11:27:24AM +0200, Vladimir Zapolskiy wrote:
> >>>> On 12/23/2016 10:32 AM, Uwe Kleine-K?nig wrote:
> >>>>> Hello,
> >>>>>
> >>>>> On Fri, Dec 23, 2016 at 10:20:20AM +0200, Vladimir Zapolskiy wrote:
> >>>>>> On 12/23/2016 03:55 AM, Guenter Roeck wrote:
> >>>>>>> What is the ultimate conclusion of this exchange ?
> >>>>>>>
> >>>>>>> Are we going to get another version of the patch, or did everyone agree that
> >>>>>>> the patch is good as it is and does not require further changes ?
> >>>>>>>
> >>>>>>
> >>>>>> I can not imagine a different fix.
> >>>>>
> >>>>> my preferred fix would be:
> >>>>>
> >>>>>  - add an imx35 compatible to all newer dtsi
> >>>>>  - update the driver to only write the wmcr on imx35 compatible devices
> >>>>>    adding only imx35.
> >>>>>
> >>>>
> >>>> It breaks old DTS vs. new kernel compatibility, I've already mentioned this.
> >>>
> >>> Correct, and I didn't deny that. In my mail I pointed out the problem
> >>> this imposes and I think it's small enough to not justify the complexity
> >>> introduced in your proposed change.
> >>>
> >>
> >> I can not statistically estimate well the severity of the problem which was
> >> fixed by commit 5fe65ce7cc (all boards with a similar change not found in
> >> a bootloader will be affected, I believe) multiplied by the rate of users,
> >> who don't update DTB.
> > 
> > I think most people updating the kernel will update the dtb at the same
> > time. And for those who don't it should be quickly obvious that
> > something is wrong during development if the machine resets
> > unexpectedly. Plus I think that most users are not affected anyhow
> > (either because their bootloader fixes up accordingly or because most
> > machines don't reset when the timer expires because the hardware isn't
> > designed accordingly). After all between introduction of i.MX35/i.MX25
> > (not later than Thu Jun 4 11:32:12 2009 +0200, commit
> > 8c25c36f33157a2e2a1fcd60b6dc00feace80631) and the broken commit
> > 5fe65ce7cc (Mon Sep 8 09:14:07 2014 +0200) it seems it wasn't an issue.
> > 
> > With my suggestion the situation on an affected machine with old dtb and
> > new kernel is just as it was in these 5 years where nobody complained.
> 
> Please feel free to send the revert of 5fe65ce7cc, if you think that
> the commit is unneeded.

I think there is at least a handful of machines that need it. And I
doubt any of them will get a new kernel without a new dtb.

> > It's fine to target a bugfree system, and if you want to target that
> > that's applaudable. But then please make it easy for others after you to
> > not undo your good and add a big comment to the compatible array of the
> > driver explaining why they are all listed explicitly and how to prevent
> > to have to expand the list further.
> 
> Please clarify, what do you mean here by "undo my good"? Revert my fix
> and break boot on i.MX31 again or something else?

What I want is that if you add all these compatibles to the driver,
please also add a big comment explaining why they are there. Otherwise
someone with good intentions will come an cleanup.
 
> > So yes, my suggestion has a risk, but I think when weighting that
> > against the overhead that is introduced in the driver, I'd go the
> > simpler way.
> 
> What overhead do you mean here? Prolonged time in a loop while finding
> a compatible by looking at 10 more values?

No, I don't think this will add relevant overhead. I want to keep the
driver simple for easier maintenance.

> That's the price of the accepted commit 5fe65ce7cc and overlooked
> incompatibilities in hardware while writing DTSI files for i.MX SoCs.
> 
> > That's of course something personal and as it's you who
> > seems to have in interest in fixing the driver, it's your way that
> > matters more. And if you document your way good enough, I won't stop
> > you.
> > 
> 
> We're going round in circles. And I don't understand what do you mean
> by "documenting my way" here.
> 
> For clarity I do NOT object against changes in DTS, I do NOT object
> to shrink the list of compatibles, I object to mix up the requested
> DTS changes for practically every i.MX powered board, bug fix and
> a new potential bug in one single change. The series of commits is
> generally good, but it lacks atomicity property of a fix, and in
> the middle of the series users have to update DTB, it is an overkill.
> 
> Is it possible to change DTS *only* and fix kernel boot problem? NO.
> Is it possible to change driver *only* and fix kernel boot problem? YES.
> Conclusion: the problem is within the driver, to solve the problem
> it is sufficient to fix the driver.

This argumentation is wrong in general. Any fixable damage can be fixed
in code only by changing how the dtb is interpreted.

The problem here is that 5fe65ce7cc is right for i.MX35 (and others) but
wrong for i.MX21 (and others) and there is no differentiation between
these two types in the driver. So I think we agree the right thing to do
in the end is: Make the driver aware that there are two different
hardware types and in the dtsi make use of the right one.


> I'll add a comment to v2:
> 
> /*
>  * The list of compatibles is added to achieve backward compatibility
>  * between DTS and kernel while fixing the incompatibility between

I'd write: "to achieve compatibility between old DTS and new kernel"

>  * i.MX21/i.MX27/i.MX31 and i.MX25/i.MX35/etc. watchdog controllers.
>  *
>  * Please do *NOT* extend the list by adding a compatible value for
>  * a controller, which is compatible with one of the already listed.
>  *
>  * You may consider to shrink the list at your own risk, but this may
>  * break backward compatibility and users may have to update their DTB,
>  * which is good eventually.
>  */
> 
> And you send the removal of the comment and compatibles as a *separate*
> change, if you find such a long list of compatibles redundant.

I'd do it the other way around and so get more and simpler patches:

 - make driver aware of i.MX35
 - fix dtsi to use i.MX35 type where applicable

and IMHO optionally:

 - introduce compatibility stuff with the above comment

I hope we converge to a solution acceptable for you and me.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v1] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs
From: Uwe Kleine-König @ 2016-12-28 20:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAOMZO5CqpfH6nq_mqPitztfSX4-oR4-61z7PnYhvOYAVnYF0yA@mail.gmail.com>

On Sat, Dec 24, 2016 at 11:00:59AM -0200, Fabio Estevam wrote:
> On Sat, Dec 10, 2016 at 11:06 PM, Vladimir Zapolskiy <vz@mleia.com> wrote:
> 
> >  static const struct of_device_id imx2_wdt_dt_ids[] = {
> > -       { .compatible = "fsl,imx21-wdt", },
> > +       { .compatible = "fsl,imx21-wdt", .data = IMX2_WDT_NO_WMCR },
> > +       { .compatible = "fsl,imx25-wdt",  },
> > +       { .compatible = "fsl,imx27-wdt", .data = IMX2_WDT_NO_WMCR },
> > +       { .compatible = "fsl,imx31-wdt", .data = IMX2_WDT_NO_WMCR },
> > +       { .compatible = "fsl,imx35-wdt",  },
> > +       { .compatible = "fsl,imx50-wdt",  },
> > +       { .compatible = "fsl,imx51-wdt",  },
> > +       { .compatible = "fsl,imx53-wdt",  },
> > +       { .compatible = "fsl,imx6q-wdt",  },
> > +       { .compatible = "fsl,imx6sl-wdt", },
> > +       { .compatible = "fsl,imx6sx-wdt", },
> > +       { .compatible = "fsl,imx6ul-wdt", },
> > +       { .compatible = "fsl,imx7d-wdt",  },
> > +       { .compatible = "fsl,vf610-wdt",  },
> >         { /* sentinel */ }
> 
> I understand this compatible list is not very nice, but in order to
> keep old dtb's working I don't see a better solution, so:
> 
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

You can at least drop fsl,imx27-wdt and fsl,imx31-wdt which then fall
back correctly to fsl,imx21-wdt.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
From: Florian Fainelli @ 2016-12-28 20:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <ae71bdc0-23c7-ce0d-7264-930ab7f405ec@gmail.com>

On 11/03/2016 10:20 AM, Florian Fainelli wrote:
> On 11/03/2016 07:16 AM, Will Deacon wrote:
>> On Wed, Nov 02, 2016 at 02:57:26PM -0700, Florian Fainelli wrote:
>>> On 11/02/2016 02:41 PM, Markus Mayer wrote:
>>>> On 2 November 2016 at 14:27, Will Deacon <will.deacon@arm.com> wrote:
>>>>> On Wed, Nov 02, 2016 at 02:07:17PM -0700, Markus Mayer wrote:
>>>>>> The question I am asking is: What do we have to lose by supporting both options?
>>>>>
>>>>> We end up passing "--fix-cortex-a53" to the linker, without knowing what it
>>>>> might do in the future.
>>>>
>>>> It seems highly unlikely that such a generic option would be added in
>>>> the future, both, because the precedent has been set for topic
>>>> specific options, and because they know it has been used in the past,
>>>> so they wouldn't add a previously used option to do something
>>>> completely different. (And if they really did, then that would be a
>>>> huge binutils bug.)
>>>>
>>>> So, we have a trade-off between a real world problem that does
>>>> currently exist and avoiding a theoretical issue that may never
>>>> materialize.
>>>
>>> Agreed, also the way Markus' patch is designed makes it such that we
>>> first try the full and current option name, and if not supported, try
>>> the second (and earlier, now obsolete) option name, so I really don't
>>> see a lot of room for things to go wrong here...
>>
>> It's not beyond the realms of possibility that ld will grow a
>> "fix-cortex-a53" option in the future, that enables all of the a53
>> workarounds. Since ld is the linker supported by the kernel and gold isn't,
>> I don't want to pass this option down.
> 
> No it's entirely reasonable to think this may happen, although:
> 
> - this has not happened yet, so once this happens, you will need to cook
> a patch for this anyway, and you will be able to gate this catch all
> linker option by an appropriate version check presumably
> 
> - you would supposedly want a fine grained set of linker options that
> are specific to workarounds you have to have enabled, instead of a catch
> all "enable all Cortex A53" workarounds
> 
>>
>> If you can't change toolchain and you want this worked around, why can't you
>> either build gold with it enabled by default, or pass the extra flag on the
>> command line to the kernel build system?
> 
> Because that creates a distribution problem and now we have to document
> this for people who want to build a kernel on their own, without
> necessarily understanding if this is something they might need, or why
> this is needed, and why the kernel is not taking care of that on its
> own? So yes, this comes down to who is responsible for what, in that
> case the kernel's Makefile is the best place where to put such knowledge
> as to which workaround needs to be enabled by the linker and it
> simplifies things a lot for people.

Was this convincing enough for Catalin to pick Markus' patch or does
that mean this patch needs to remain out of tree for us because of using
a slightly older toolchain?

Thanks
-- 
Florian

^ permalink raw reply

* [PATCH] crypto: arm/aes-neonbs - process 8 blocks in parallel if we can
From: Ard Biesheuvel @ 2016-12-28 19:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161228092328.GA12494@gondor.apana.org.au>

On 28 December 2016 at 09:23, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Wed, Dec 28, 2016 at 09:19:32AM +0000, Ard Biesheuvel wrote:
>>
>> Ok, so that implies a field in the skcipher algo struct then, rather than some definition internal to the driver?
>
> Oh yes it should definitely be visible to other crypto API drivers
> and algorithms.  It's just that we don't want to export it outside
> of the crypto API, e.g., to IPsec or algif.
>

Understood.

So about this chunksize, is it ever expected to assume other values
than 1 (for stream ciphers) or the block size (for block ciphers)?
Having block size, IV size *and* chunk size fields may be confusing to
some already, so if the purpose of chunk size can be fulfilled by a
single 'stream cipher' flag, perhaps we should change that first.

^ permalink raw reply

* [PATCH 7/8] ARM: s3c64xx: Drop initialization of unused struct s3c_audio_pdata fields
From: Krzysztof Kozlowski @ 2016-12-28 17:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1478791076-19528-8-git-send-email-s.nawrocki@samsung.com>

On Thu, Nov 10, 2016 at 04:17:55PM +0100, Sylwester Nawrocki wrote:
> Remove initialization of dma_{filter, playback, capture, capture_mic}
> fields where it is not used any more.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
>  arch/arm/mach-s3c64xx/dev-audio.c | 19 -------------------
>  1 file changed, 19 deletions(-)
>

Sylwester,

This and 8/8 should be safe to apply, right?

BR,
Krzysztof

^ permalink raw reply

* [RESEND v2] dt-bindings: video: exynos7-decon: Remove obsolete samsung, power-domain property
From: Krzysztof Kozlowski @ 2016-12-28 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

The samsung,power-domain property is obsolete since commit 0da658704136
("ARM: dts: convert to generic power domain bindings for exynos DT").
Replace it with generic one.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Rob Herring <robh@kernel.org>

---

Changes since v1:
1. Just add the acks/reviews.

I though it will be applied by Exynos DRM maintainer but there was no
response.
Dear Rob, could you pick it up?
---
 Documentation/devicetree/bindings/display/exynos/exynos7-decon.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/exynos/exynos7-decon.txt b/Documentation/devicetree/bindings/display/exynos/exynos7-decon.txt
index 3938caacf11c..8346fb18a358 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos7-decon.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos7-decon.txt
@@ -33,7 +33,7 @@ Required properties:
 - i80-if-timings: timing configuration for lcd i80 interface support.
 
 Optional Properties:
-- samsung,power-domain: a phandle to DECON power domain node.
+- power-domains: a phandle to DECON power domain node.
 - display-timings: timing settings for DECON, as described in document [1].
 		Can be used in case timings cannot be provided otherwise
 		or to override timings provided by the panel.
-- 
2.9.3

^ permalink raw reply related

* [PATCHv2 net-next 00/16] net: mvpp2: add basic support for PPv2.2
From: David Miller @ 2016-12-28 17:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Wed, 28 Dec 2016 17:46:16 +0100

> This series depends on the series named "net: mvpp2: misc improvements
> and preparation patches".

Please in the future only submit one patch series at a time.

If I've told you that a large patch series is hard to review and that
therefore one should keep each submitted series small and to a
reasonable size, that is completely undermined when you submit
multiple series to work around that request.

Thank you.

^ permalink raw reply

* [PATCHv2 net-next 16/16] net: mvpp2: finally add the PPv2.2 compatible string
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

Now that the mvpp2 driver has been modified to accommodate the support
for PPv2.2, we can finally advertise this support by adding the
appropriate compatible string.

At the same time, we update the Kconfig description of the MVPP2 driver.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/Kconfig | 4 ++--
 drivers/net/ethernet/marvell/mvpp2.c | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index d2555e8b..da6fb82 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -82,13 +82,13 @@ config MVNETA_BM
 	  that all dependencies are met.
 
 config MVPP2
-	tristate "Marvell Armada 375 network interface support"
+	tristate "Marvell Armada 375/7K/8K network interface support"
 	depends on ARCH_MVEBU || COMPILE_TEST
 	depends on HAS_DMA
 	select MVMDIO
 	---help---
 	  This driver supports the network interface units in the
-	  Marvell ARMADA 375 SoC.
+	  Marvell ARMADA 375, 7K and 8K SoCs.
 
 config PXA168_ETH
 	tristate "Marvell pxa168 ethernet support"
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 194de00..9e744d2 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -6997,6 +6997,10 @@ static const struct of_device_id mvpp2_match[] = {
 		.compatible = "marvell,armada-375-pp2",
 		.data = (void *)MVPP21,
 	},
+	{
+		.compatible = "marvell,armada-7k-pp22",
+		.data = (void *)MVPP22,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mvpp2_match);
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 15/16] net: mvpp2: add support for an additional clock needed for PPv2.2
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

The PPv2.2 variant of the network controller needs an additional
clock, the "MG clock" in order for the IP block to operate
properly. This commit adds support for this additional clock to the
driver, reworking as needed the error handling path.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 20e9429..194de00 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -702,6 +702,7 @@ struct mvpp2 {
 	/* Common clocks */
 	struct clk *pp_clk;
 	struct clk *gop_clk;
+	struct clk *mg_clk;
 
 	/* List of pointers to port structures */
 	struct mvpp2_port **port_list;
@@ -6899,6 +6900,18 @@ static int mvpp2_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_pp_clk;
 
+	if (priv->hw_version == MVPP22) {
+		priv->mg_clk = devm_clk_get(&pdev->dev, "mg_clk");
+		if (IS_ERR(priv->mg_clk)) {
+			err = PTR_ERR(priv->mg_clk);
+			goto err_gop_clk;
+		}
+
+		err = clk_prepare_enable(priv->mg_clk);
+		if (err < 0)
+			goto err_gop_clk;
+	}
+
 	/* Get system's tclk rate */
 	priv->tclk = clk_get_rate(priv->pp_clk);
 
@@ -6906,14 +6919,14 @@ static int mvpp2_probe(struct platform_device *pdev)
 	err = mvpp2_init(pdev, priv);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to initialize controller\n");
-		goto err_gop_clk;
+		goto err_mg_clk;
 	}
 
 	port_count = of_get_available_child_count(dn);
 	if (port_count == 0) {
 		dev_err(&pdev->dev, "no ports enabled\n");
 		err = -ENODEV;
-		goto err_gop_clk;
+		goto err_mg_clk;
 	}
 
 	priv->port_list = devm_kcalloc(&pdev->dev, port_count,
@@ -6921,19 +6934,22 @@ static int mvpp2_probe(struct platform_device *pdev)
 				      GFP_KERNEL);
 	if (!priv->port_list) {
 		err = -ENOMEM;
-		goto err_gop_clk;
+		goto err_mg_clk;
 	}
 
 	/* Initialize ports */
 	for_each_available_child_of_node(dn, port_node) {
 		err = mvpp2_port_probe(pdev, port_node, priv);
 		if (err < 0)
-			goto err_gop_clk;
+			goto err_mg_clk;
 	}
 
 	platform_set_drvdata(pdev, priv);
 	return 0;
 
+err_mg_clk:
+	if (priv->hw_version == MVPP22)
+		clk_disable_unprepare(priv->mg_clk);
 err_gop_clk:
 	clk_disable_unprepare(priv->gop_clk);
 err_pp_clk:
@@ -6969,6 +6985,7 @@ static int mvpp2_remove(struct platform_device *pdev)
 				  aggr_txq->descs_phys);
 	}
 
+	clk_disable_unprepare(priv->mg_clk);
 	clk_disable_unprepare(priv->pp_clk);
 	clk_disable_unprepare(priv->gop_clk);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 14/16] net: mvpp2: adapt rxq distribution to PPv2.2
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

In PPv2.1, we have a maximum of 8 RXQs per port, with a default of 4
RXQs per port, and we were assigning RXQs 0->3 to the first port, 4->7
to the second port, 8->11 to the third port, etc.

In PPv2.2, we have a maximum of 32 RXQs per port, and we must allocate
RXQs from the range of 32 RXQs available for each port. So port 0 must
use RXQs in the range 0->31, port 1 in the range 32->63, etc.

This commit adapts the mvpp2 to this difference between PPv2.1 and
PPv2.2:

 - The constant definition MVPP2_MAX_RXQ is replaced by a new field
   'max_port_rxqs' in 'struct mvpp2', which stores the maximum number of
   RXQs per port. This field is initialized during ->probe() depending
   on the IP version.

 - MVPP2_RXQ_TOTAL_NUM is removed, and instead we calculate the total
   number of RXQs by multiplying the number of ports by the maximum of
   RXQs per port. This was anyway used in only one place.

 - In mvpp2_port_probe(), the calculation of port->first_rxq is adjusted
   to cope with the different allocation strategy between PPv2.1 and
   PPv2.2. Due to this change, the 'next_first_rxq' argument of this
   function is no longer needed and is removed.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index baad991..20e9429 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -399,15 +399,9 @@
 /* Maximum number of TXQs used by single port */
 #define MVPP2_MAX_TXQ			8
 
-/* Maximum number of RXQs used by single port */
-#define MVPP2_MAX_RXQ			8
-
 /* Dfault number of RXQs in use */
 #define MVPP2_DEFAULT_RXQ		4
 
-/* Total number of RXQs available to all ports */
-#define MVPP2_RXQ_TOTAL_NUM		(MVPP2_MAX_PORTS * MVPP2_MAX_RXQ)
-
 /* Max number of Rx descriptors */
 #define MVPP2_MAX_RXD			128
 
@@ -728,6 +722,9 @@ struct mvpp2 {
 
 	/* HW version */
 	enum { MVPP21, MVPP22 } hw_version;
+
+	/* Maximum number of RXQs per port */
+	unsigned int max_port_rxqs;
 };
 
 struct mvpp2_pcpu_stats {
@@ -6333,7 +6330,8 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 	struct mvpp2_txq_pcpu *txq_pcpu;
 	int queue, cpu, err;
 
-	if (port->first_rxq + rxq_number > MVPP2_RXQ_TOTAL_NUM)
+	if (port->first_rxq + rxq_number >
+	    MVPP2_MAX_PORTS * priv->max_port_rxqs)
 		return -EINVAL;
 
 	/* Disable port */
@@ -6452,8 +6450,7 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 /* Ports initialization */
 static int mvpp2_port_probe(struct platform_device *pdev,
 			    struct device_node *port_node,
-			    struct mvpp2 *priv,
-			    int *next_first_rxq)
+			    struct mvpp2 *priv)
 {
 	struct device_node *phy_node;
 	struct mvpp2_port *port;
@@ -6511,7 +6508,11 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 
 	port->priv = priv;
 	port->id = id;
-	port->first_rxq = *next_first_rxq;
+	if (priv->hw_version == MVPP21)
+		port->first_rxq = port->id * rxq_number;
+	else
+		port->first_rxq = port->id * priv->max_port_rxqs;
+
 	port->phy_node = phy_node;
 	port->phy_interface = phy_mode;
 
@@ -6608,8 +6609,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	}
 	netdev_info(dev, "Using %s mac address %pM\n", mac_from, dev->dev_addr);
 
-	/* Increment the first Rx queue number to be used by the next port */
-	*next_first_rxq += rxq_number;
 	priv->port_list[id] = port;
 	return 0;
 
@@ -6755,7 +6754,7 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	u32 val;
 
 	/* Checks for hardware constraints */
-	if (rxq_number % 4 || (rxq_number > MVPP2_MAX_RXQ) ||
+	if (rxq_number % 4 || (rxq_number > priv->max_port_rxqs) ||
 	    (txq_number > MVPP2_MAX_TXQ)) {
 		dev_err(&pdev->dev, "invalid queue size parameter\n");
 		return -EINVAL;
@@ -6844,7 +6843,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 	struct device_node *port_node;
 	struct mvpp2 *priv;
 	struct resource *res;
-	int port_count, first_rxq, cpu;
+	int port_count, cpu;
 	int err;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct mvpp2), GFP_KERNEL);
@@ -6879,6 +6878,11 @@ static int mvpp2_probe(struct platform_device *pdev)
 		priv->cpu_base[cpu] = priv->base + cpu * addr_space_sz;
 	}
 
+	if (priv->hw_version == MVPP21)
+		priv->max_port_rxqs = 8;
+	else
+		priv->max_port_rxqs = 32;
+
 	priv->pp_clk = devm_clk_get(&pdev->dev, "pp_clk");
 	if (IS_ERR(priv->pp_clk))
 		return PTR_ERR(priv->pp_clk);
@@ -6921,9 +6925,8 @@ static int mvpp2_probe(struct platform_device *pdev)
 	}
 
 	/* Initialize ports */
-	first_rxq = 0;
 	for_each_available_child_of_node(dn, port_node) {
-		err = mvpp2_port_probe(pdev, port_node, priv, &first_rxq);
+		err = mvpp2_port_probe(pdev, port_node, priv);
 		if (err < 0)
 			goto err_gop_clk;
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 13/16] net: mvpp2: rework RXQ interrupt group initialization for PPv2.2
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

This commit adjusts how the MVPP2_ISR_RXQ_GROUP_REG register is
configured, since it changed between PPv2.1 and PPv2.2.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 45 ++++++++++++++++++++++++++++++++----
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index d5b197d..baad991 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -188,7 +188,21 @@
 /* Interrupt Cause and Mask registers */
 #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)		(0x5200 + 4 * (rxq))
 #define     MVPP2_MAX_ISR_RX_THRESHOLD		0xfffff0
-#define MVPP2_ISR_RXQ_GROUP_REG(rxq)		(0x5400 + 4 * (rxq))
+#define MVPP21_ISR_RXQ_GROUP_REG(rxq)		(0x5400 + 4 * (rxq))
+
+#define MVPP22_ISR_RXQ_GROUP_INDEX_REG          0x5400
+#define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf
+#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_MASK   0x380
+#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET 7
+
+#define MVPP22_ISR_RXQ_GROUP_INDEX_SUBGROUP_MASK 0xf
+#define MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_MASK   0x380
+
+#define MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG     0x5404
+#define MVPP22_ISR_RXQ_SUB_GROUP_STARTQ_MASK    0x1f
+#define MVPP22_ISR_RXQ_SUB_GROUP_SIZE_MASK      0xf00
+#define MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET    8
+
 #define MVPP2_ISR_ENABLE_REG(port)		(0x5420 + 4 * (port))
 #define     MVPP2_ISR_ENABLE_INTERRUPT(mask)	((mask) & 0xffff)
 #define     MVPP2_ISR_DISABLE_INTERRUPT(mask)	(((mask) << 16) & 0xffff0000)
@@ -6385,7 +6399,18 @@ static int mvpp2_port_init(struct mvpp2_port *port)
 	}
 
 	/* Configure Rx queue group interrupt for this port */
-	mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(port->id), rxq_number);
+	if (priv->hw_version == MVPP21)
+		mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(port->id),
+			    rxq_number);
+	else {
+		u32 val;
+
+		val = (port->id << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET);
+		mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val);
+
+		val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET);
+		mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val);
+	}
 
 	/* Create Rx descriptor rings */
 	for (queue = 0; queue < rxq_number; queue++) {
@@ -6775,8 +6800,20 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	mvpp2_rx_fifo_init(priv);
 
 	/* Reset Rx queue group interrupt configuration */
-	for (i = 0; i < MVPP2_MAX_PORTS; i++)
-		mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(i), rxq_number);
+	for (i = 0; i < MVPP2_MAX_PORTS; i++) {
+		if (priv->hw_version == MVPP21)
+			mvpp2_write(priv, MVPP21_ISR_RXQ_GROUP_REG(i),
+				    rxq_number);
+		else {
+			u32 val;
+
+			val = (i << MVPP22_ISR_RXQ_GROUP_INDEX_GROUP_OFFSET);
+			mvpp2_write(priv, MVPP22_ISR_RXQ_GROUP_INDEX_REG, val);
+
+			val = (rxq_number << MVPP22_ISR_RXQ_SUB_GROUP_SIZE_OFFSET);
+			mvpp2_write(priv, MVPP22_ISR_RXQ_SUB_GROUP_CONFIG_REG, val);
+		}
+	}
 
 	if (priv->hw_version == MVPP21)
 		writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT,
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 12/16] net: mvpp2: add AXI bridge initialization for PPv2.2
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

The PPv2.2 unit is connected to an AXI bus on Armada 7K/8K, so this
commit adds the necessary initialization of the AXI bridge.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 85 ++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index eb55576..d5b197d 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -157,6 +157,34 @@
 #define MVPP2_WIN_REMAP(w)			(0x4040 + ((w) << 2))
 #define MVPP2_BASE_ADDR_ENABLE			0x4060
 
+/* AXI Bridge Registers */
+#define MVPP22_AXI_BM_WR_ATTR_REG		0x4100
+#define MVPP22_AXI_BM_RD_ATTR_REG		0x4104
+#define MVPP22_AXI_AGGRQ_DESCR_RD_ATTR_REG	0x4110
+#define MVPP22_AXI_TXQ_DESCR_WR_ATTR_REG	0x4114
+#define MVPP22_AXI_TXQ_DESCR_RD_ATTR_REG	0x4118
+#define MVPP22_AXI_RXQ_DESCR_WR_ATTR_REG	0x411c
+#define MVPP22_AXI_RX_DATA_WR_ATTR_REG		0x4120
+#define MVPP22_AXI_TX_DATA_RD_ATTR_REG		0x4130
+#define MVPP22_AXI_RD_NORMAL_CODE_REG		0x4150
+#define MVPP22_AXI_RD_SNOOP_CODE_REG		0x4154
+#define MVPP22_AXI_WR_NORMAL_CODE_REG		0x4160
+#define MVPP22_AXI_WR_SNOOP_CODE_REG		0x4164
+
+/* Values for AXI Bridge registers */
+#define MVPP22_AXI_ATTR_CACHE_OFFS		0
+#define MVPP22_AXI_ATTR_DOMAIN_OFFS		12
+
+#define MVPP22_AXI_CODE_CACHE_OFFS		0
+#define MVPP22_AXI_CODE_DOMAIN_OFFS		4
+
+#define MVPP22_AXI_CODE_CACHE_NON_CACHE		0x3
+#define MVPP22_AXI_CODE_CACHE_WR_CACHE		0x7
+#define MVPP22_AXI_CODE_CACHE_RD_CACHE		0xb
+
+#define MVPP22_AXI_CODE_DOMAIN_OUTER_DOM	2
+#define MVPP22_AXI_CODE_DOMAIN_SYSTEM		3
+
 /* Interrupt Cause and Mask registers */
 #define MVPP2_ISR_RX_THRESHOLD_REG(rxq)		(0x5200 + 4 * (rxq))
 #define     MVPP2_MAX_ISR_RX_THRESHOLD		0xfffff0
@@ -6640,6 +6668,60 @@ static void mvpp2_rx_fifo_init(struct mvpp2 *priv)
 	mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1);
 }
 
+static void mvpp2_axi_init(struct mvpp2 *priv)
+{
+	u32 val, rdval, wrval;
+
+	mvpp2_write(priv, MVPP22_BM_ADDR_HIGH_RLS_REG, 0x0);
+
+	/* AXI Bridge Configuration */
+
+	rdval = MVPP22_AXI_CODE_CACHE_RD_CACHE
+		<< MVPP22_AXI_ATTR_CACHE_OFFS;
+	rdval |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_ATTR_DOMAIN_OFFS;
+
+	wrval = MVPP22_AXI_CODE_CACHE_WR_CACHE
+		<< MVPP22_AXI_ATTR_CACHE_OFFS;
+	wrval |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_ATTR_DOMAIN_OFFS;
+
+	/* BM */
+	mvpp2_write(priv, MVPP22_AXI_BM_WR_ATTR_REG, wrval);
+	mvpp2_write(priv, MVPP22_AXI_BM_RD_ATTR_REG, rdval);
+
+	/* Descriptors */
+	mvpp2_write(priv, MVPP22_AXI_AGGRQ_DESCR_RD_ATTR_REG, rdval);
+	mvpp2_write(priv, MVPP22_AXI_TXQ_DESCR_WR_ATTR_REG, wrval);
+	mvpp2_write(priv, MVPP22_AXI_TXQ_DESCR_RD_ATTR_REG, rdval);
+	mvpp2_write(priv, MVPP22_AXI_RXQ_DESCR_WR_ATTR_REG, wrval);
+
+	/* Buffer Data */
+	mvpp2_write(priv, MVPP22_AXI_TX_DATA_RD_ATTR_REG, rdval);
+	mvpp2_write(priv, MVPP22_AXI_RX_DATA_WR_ATTR_REG, wrval);
+
+	val = MVPP22_AXI_CODE_CACHE_NON_CACHE
+		<< MVPP22_AXI_CODE_CACHE_OFFS;
+	val |= MVPP22_AXI_CODE_DOMAIN_SYSTEM
+		<< MVPP22_AXI_CODE_DOMAIN_OFFS;
+	mvpp2_write(priv, MVPP22_AXI_RD_NORMAL_CODE_REG, val);
+	mvpp2_write(priv, MVPP22_AXI_WR_NORMAL_CODE_REG, val);
+
+	val = MVPP22_AXI_CODE_CACHE_RD_CACHE
+		<< MVPP22_AXI_CODE_CACHE_OFFS;
+	val |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_CODE_DOMAIN_OFFS;
+
+	mvpp2_write(priv, MVPP22_AXI_RD_SNOOP_CODE_REG, val);
+
+	val = MVPP22_AXI_CODE_CACHE_WR_CACHE
+		<< MVPP22_AXI_CODE_CACHE_OFFS;
+	val |= MVPP22_AXI_CODE_DOMAIN_OUTER_DOM
+		<< MVPP22_AXI_CODE_DOMAIN_OFFS;
+
+	mvpp2_write(priv, MVPP22_AXI_WR_SNOOP_CODE_REG, val);
+}
+
 /* Initialize network controller common part HW */
 static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 {
@@ -6659,6 +6741,9 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	if (dram_target_info)
 		mvpp2_conf_mbus_windows(dram_target_info, priv);
 
+	if (priv->hw_version == MVPP22)
+		mvpp2_axi_init(priv);
+
 	/* Disable HW PHY polling */
 	if (priv->hw_version == MVPP21) {
 		val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 11/16] net: mvpp2: handle misc PPv2.1/PPv2.2 differences
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

This commit handles a few miscellaneous differences between PPv2.1 and
PPv2.2 in different areas, where code done for PPv2.1 doesn't apply for
PPv2.2 or needs to be adjusted (getting the MAC address, disabling PHY
polling, etc.).

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 389cc62..eb55576 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -304,6 +304,9 @@
 #define      MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(v)	(((v) << 6) & \
 					MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK)
 
+#define MVPP22_SMI_MISC_CFG_REG			0x2a204
+#define      MVPP22_SMI_POLLING_EN		BIT(10)
+
 #define MVPP22_PORT_BASE			0x30e00
 #define MVPP22_PORT_OFFSET			0x1000
 
@@ -5823,7 +5826,7 @@ static int mvpp2_check_ringparam_valid(struct net_device *dev,
 	return 0;
 }
 
-static void mvpp2_get_mac_address(struct mvpp2_port *port, unsigned char *addr)
+static void mvpp21_get_mac_address(struct mvpp2_port *port, unsigned char *addr)
 {
 	u32 mac_addr_l, mac_addr_m, mac_addr_h;
 
@@ -6272,7 +6275,7 @@ static const struct ethtool_ops mvpp2_eth_tool_ops = {
 
 /* Driver initialization */
 
-static void mvpp2_port_power_up(struct mvpp2_port *port)
+static void mvpp21_port_power_up(struct mvpp2_port *port)
 {
 	mvpp2_port_mii_set(port);
 	mvpp2_port_periodic_xon_disable(port);
@@ -6491,7 +6494,8 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		mac_from = "device tree";
 		ether_addr_copy(dev->dev_addr, dt_mac_addr);
 	} else {
-		mvpp2_get_mac_address(port, hw_mac_addr);
+		if (priv->hw_version == MVPP21)
+			mvpp21_get_mac_address(port, hw_mac_addr);
 		if (is_valid_ether_addr(hw_mac_addr)) {
 			mac_from = "hardware";
 			ether_addr_copy(dev->dev_addr, hw_mac_addr);
@@ -6511,7 +6515,9 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 		dev_err(&pdev->dev, "failed to init port %d\n", id);
 		goto err_free_stats;
 	}
-	mvpp2_port_power_up(port);
+
+	if (priv->hw_version == MVPP21)
+		mvpp21_port_power_up(port);
 
 	port->pcpu = alloc_percpu(struct mvpp2_port_pcpu);
 	if (!port->pcpu) {
@@ -6654,9 +6660,15 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 		mvpp2_conf_mbus_windows(dram_target_info, priv);
 
 	/* Disable HW PHY polling */
-	val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
-	val |= MVPP2_PHY_AN_STOP_SMI0_MASK;
-	writel(val, priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
+	if (priv->hw_version == MVPP21) {
+		val = readl(priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
+		val |= MVPP2_PHY_AN_STOP_SMI0_MASK;
+		writel(val, priv->lms_base + MVPP2_PHY_AN_CFG0_REG);
+	} else {
+		val = readl(priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
+		val &= ~MVPP22_SMI_POLLING_EN;
+		writel(val, priv->iface_base + MVPP22_SMI_MISC_CFG_REG);
+	}
 
 	/* Allocate and initialize aggregated TXQs */
 	priv->aggr_txqs = devm_kcalloc(&pdev->dev, num_present_cpus(),
@@ -6681,8 +6693,9 @@ static int mvpp2_init(struct platform_device *pdev, struct mvpp2 *priv)
 	for (i = 0; i < MVPP2_MAX_PORTS; i++)
 		mvpp2_write(priv, MVPP2_ISR_RXQ_GROUP_REG(i), rxq_number);
 
-	writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT,
-	       priv->lms_base + MVPP2_MNG_EXTENDED_GLOBAL_CTRL_REG);
+	if (priv->hw_version == MVPP21)
+		writel(MVPP2_EXT_GLOBAL_CTRL_DEFAULT,
+		       priv->lms_base + MVPP2_MNG_EXTENDED_GLOBAL_CTRL_REG);
 
 	/* Allow cache snoop when transmiting packets */
 	mvpp2_write(priv, MVPP2_TX_SNOOP_REG, 0x1);
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 10/16] net: mvpp2: handle register mapping and access for PPv2.2
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

This commit adjusts the mvpp2 driver register mapping and access logic
to support PPv2.2, to handle a number of differences.

Due to how the registers are laid out in memory, the Device Tree binding
for the "reg" property is different:

 - On PPv2.1, we had a first area for the common registers, and then one
   area per port.

 - On PPv2.2, we have a first area for the common registers, and a
   second area for all the per-ports registers.

In addition, on PPv2.2, the area for the common registers is split into
so-called "address spaces" of 64 KB each. They allow to access the same
registers, but from different CPUs. Hence the introduction of cpu_base[]
in 'struct mvpp2', and the modification of the mvpp2_write() and
mvpp2_read() register accessors. For PPv2.1, the compatibility is
preserved by using an "address space" size of 0.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 78 +++++++++++++++++++++++++++++-------
 1 file changed, 64 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 22f7970..389cc62 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -304,6 +304,9 @@
 #define      MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(v)	(((v) << 6) & \
 					MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK)
 
+#define MVPP22_PORT_BASE			0x30e00
+#define MVPP22_PORT_OFFSET			0x1000
+
 #define MVPP2_CAUSE_TXQ_SENT_DESC_ALL_MASK	0xff
 
 /* Descriptor ring Macros */
@@ -631,6 +634,11 @@ enum mvpp2_prs_l3_cast {
  */
 #define MVPP2_BM_SHORT_PKT_SIZE		MVPP2_RX_MAX_PKT_SIZE(512)
 
+#define MVPP21_ADDR_SPACE_SZ		0
+#define MVPP22_ADDR_SPACE_SZ		SZ_64K
+
+#define MVPP2_MAX_CPUS			4
+
 enum mvpp2_bm_type {
 	MVPP2_BM_FREE,
 	MVPP2_BM_SWF_LONG,
@@ -644,6 +652,13 @@ struct mvpp2 {
 	/* Shared registers' base addresses */
 	void __iomem *base;
 	void __iomem *lms_base;
+	void __iomem *iface_base;
+
+	/* On PPv2.2, each CPU can access the base register through a
+	 * separate address space, each 64 KB apart from each
+	 * other.
+	 */
+	void __iomem *cpu_base[MVPP2_MAX_CPUS];
 
 	/* Common clocks */
 	struct clk *pp_clk;
@@ -1021,12 +1036,21 @@ static int txq_number = MVPP2_MAX_TXQ;
 
 static void mvpp2_write(struct mvpp2 *priv, u32 offset, u32 data)
 {
-	writel(data, priv->base + offset);
+	int cpu = get_cpu();
+
+	writel(data, priv->cpu_base[cpu] + offset);
+	put_cpu();
 }
 
 static u32 mvpp2_read(struct mvpp2 *priv, u32 offset)
 {
-	return readl(priv->base + offset);
+	int cpu = get_cpu();
+	u32 val;
+
+	val = readl(priv->cpu_base[cpu] + offset);
+	put_cpu();
+
+	return val;
 }
 
 static dma_addr_t mvpp2_txdesc_phys_addr_get(struct mvpp2_port *port,
@@ -6386,7 +6410,6 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	u32 id;
 	int features;
 	int phy_mode;
-	int priv_common_regs_num = 2;
 	int err, i, cpu;
 
 	dev = alloc_etherdev_mqs(sizeof(struct mvpp2_port), txq_number,
@@ -6436,12 +6459,24 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 	port->phy_node = phy_node;
 	port->phy_interface = phy_mode;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM,
-				    priv_common_regs_num + id);
-	port->base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(port->base)) {
-		err = PTR_ERR(port->base);
-		goto err_free_irq;
+	if (priv->hw_version == MVPP21) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 2 + id);
+		port->base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(port->base)) {
+			err = PTR_ERR(port->base);
+			goto err_free_irq;
+		}
+	} else {
+		u32 gop_id;
+
+		if (of_property_read_u32(port_node, "gop-port-id", &gop_id)) {
+			err = -EINVAL;
+			dev_err(&pdev->dev, "missing gop-port-id value\n");
+			goto err_free_irq;
+		}
+
+		port->base = priv->iface_base + MVPP22_PORT_BASE +
+			gop_id * MVPP22_PORT_OFFSET;
 	}
 
 	/* Alloc per-cpu stats */
@@ -6674,7 +6709,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 	struct device_node *port_node;
 	struct mvpp2 *priv;
 	struct resource *res;
-	int port_count, first_rxq;
+	int port_count, first_rxq, cpu;
 	int err;
 
 	priv = devm_kzalloc(&pdev->dev, sizeof(struct mvpp2), GFP_KERNEL);
@@ -6689,10 +6724,25 @@ static int mvpp2_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->base))
 		return PTR_ERR(priv->base);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	priv->lms_base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(priv->lms_base))
-		return PTR_ERR(priv->lms_base);
+	if (priv->hw_version == MVPP21) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		priv->lms_base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->lms_base))
+			return PTR_ERR(priv->lms_base);
+	} else {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		priv->iface_base = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->iface_base))
+			return PTR_ERR(priv->iface_base);
+	}
+
+	for_each_present_cpu(cpu) {
+		u32 addr_space_sz;
+
+		addr_space_sz = (priv->hw_version == MVPP21 ?
+				 MVPP21_ADDR_SPACE_SZ : MVPP22_ADDR_SPACE_SZ);
+		priv->cpu_base[cpu] = priv->base + cpu * addr_space_sz;
+	}
 
 	priv->pp_clk = devm_clk_get(&pdev->dev, "pp_clk");
 	if (IS_ERR(priv->pp_clk))
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 09/16] net: mvpp2: adjust mvpp2_{rxq, txq}_init for PPv2.2
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

In PPv2.2, the MVPP2_RXQ_DESC_ADDR_REG and MVPP2_TXQ_DESC_ADDR_REG
registers have a slightly different layout, because they need to contain
a 64-bit address for the RX and TX descriptor arrays. This commit
adjusts those functions accordingly.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 23f2368..22f7970 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -102,6 +102,7 @@
 /* Descriptor Manager Top Registers */
 #define MVPP2_RXQ_NUM_REG			0x2040
 #define MVPP2_RXQ_DESC_ADDR_REG			0x2044
+#define     MVPP22_DESC_ADDR_OFFS		8
 #define MVPP2_RXQ_DESC_SIZE_REG			0x2048
 #define     MVPP2_RXQ_DESC_SIZE_MASK		0x3ff0
 #define MVPP2_RXQ_STATUS_UPDATE_REG(rxq)	(0x3000 + 4 * (rxq))
@@ -143,6 +144,7 @@
 #define MVPP2_TXQ_RSVD_CLR_REG			0x20b8
 #define     MVPP2_TXQ_RSVD_CLR_OFFSET		16
 #define MVPP2_AGGR_TXQ_DESC_ADDR_REG(cpu)	(0x2100 + 4 * (cpu))
+#define     MVPP22_AGGR_TXQ_DESC_ADDR_OFFS	8
 #define MVPP2_AGGR_TXQ_DESC_SIZE_REG(cpu)	(0x2140 + 4 * (cpu))
 #define     MVPP2_AGGR_TXQ_DESC_SIZE_MASK	0x3ff0
 #define MVPP2_AGGR_TXQ_STATUS_REG(cpu)		(0x2180 + 4 * (cpu))
@@ -4769,6 +4771,8 @@ static int mvpp2_aggr_txq_init(struct platform_device *pdev,
 			       int desc_num, int cpu,
 			       struct mvpp2 *priv)
 {
+	u32 txq_phys;
+
 	/* Allocate memory for TX descriptors */
 	aggr_txq->descs = dma_alloc_coherent(&pdev->dev,
 				desc_num * MVPP2_DESC_ALIGNED_SIZE,
@@ -4782,10 +4786,16 @@ static int mvpp2_aggr_txq_init(struct platform_device *pdev,
 	aggr_txq->next_desc_to_proc = mvpp2_read(priv,
 						 MVPP2_AGGR_TXQ_INDEX_REG(cpu));
 
-	/* Set Tx descriptors queue starting address */
-	/* indirect access */
-	mvpp2_write(priv, MVPP2_AGGR_TXQ_DESC_ADDR_REG(cpu),
-		    aggr_txq->descs_phys);
+	/* Set Tx descriptors queue starting address indirect
+	 * access
+	 */
+	if (priv->hw_version == MVPP21)
+		txq_phys = aggr_txq->descs_phys;
+	else
+		txq_phys = aggr_txq->descs_phys >>
+			MVPP22_AGGR_TXQ_DESC_ADDR_OFFS;
+
+	mvpp2_write(priv, MVPP2_AGGR_TXQ_DESC_ADDR_REG(cpu), txq_phys);
 	mvpp2_write(priv, MVPP2_AGGR_TXQ_DESC_SIZE_REG(cpu), desc_num);
 
 	return 0;
@@ -4796,6 +4806,8 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
 			  struct mvpp2_rx_queue *rxq)
 
 {
+	u32 rxq_phys;
+
 	rxq->size = port->rx_ring_size;
 
 	/* Allocate memory for RX descriptors */
@@ -4812,7 +4824,11 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
 
 	/* Set Rx descriptors queue starting address - indirect access */
 	mvpp2_write(port->priv, MVPP2_RXQ_NUM_REG, rxq->id);
-	mvpp2_write(port->priv, MVPP2_RXQ_DESC_ADDR_REG, rxq->descs_phys);
+	if (port->priv->hw_version == MVPP21)
+		rxq_phys = rxq->descs_phys;
+	else
+		rxq_phys = rxq->descs_phys >> MVPP22_DESC_ADDR_OFFS;
+	mvpp2_write(port->priv, MVPP2_RXQ_DESC_ADDR_REG, rxq_phys);
 	mvpp2_write(port->priv, MVPP2_RXQ_DESC_SIZE_REG, rxq->size);
 	mvpp2_write(port->priv, MVPP2_RXQ_INDEX_REG, 0);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCHv2 net-next 08/16] net: mvpp2: adapt mvpp2_defaults_set() to PPv2.2
From: Thomas Petazzoni @ 2016-12-28 16:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1482943592-12556-1-git-send-email-thomas.petazzoni@free-electrons.com>

This commit modifies the mvpp2_defaults_set() function to not do the
loopback and FIFO threshold initialization, which are not needed for
PPv2.2.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/net/ethernet/marvell/mvpp2.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index 8fc818d..23f2368 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -4205,16 +4205,18 @@ static void mvpp2_defaults_set(struct mvpp2_port *port)
 {
 	int tx_port_num, val, queue, ptxq, lrxq;
 
-	/* Configure port to loopback if needed */
-	if (port->flags & MVPP2_F_LOOPBACK)
-		mvpp2_port_loopback_set(port);
-
-	/* Update TX FIFO MIN Threshold */
-	val = readl(port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
-	val &= ~MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK;
-	/* Min. TX threshold must be less than minimal packet length */
-	val |= MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(64 - 4 - 2);
-	writel(val, port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
+	if (port->priv->hw_version == MVPP21) {
+		/* Configure port to loopback if needed */
+		if (port->flags & MVPP2_F_LOOPBACK)
+			mvpp2_port_loopback_set(port);
+
+		/* Update TX FIFO MIN Threshold */
+		val = readl(port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
+		val &= ~MVPP2_GMAC_TX_FIFO_MIN_TH_ALL_MASK;
+		/* Min. TX threshold must be less than minimal packet length */
+		val |= MVPP2_GMAC_TX_FIFO_MIN_TH_MASK(64 - 4 - 2);
+		writel(val, port->base + MVPP2_GMAC_PORT_FIFO_CFG_1_REG);
+	}
 
 	/* Disable Legacy WRR, Disable EJP, Release from reset */
 	tx_port_num = mvpp2_egress_port(port);
-- 
2.7.4

^ permalink raw reply related


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