Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 4/5] arm64: dts: allwinner: h6: Use macros for R_CCU clock and reset indices
From: Icenowy Zheng @ 2018-06-20 13:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180620051540.25617-5-wens@csie.org>

? 2018-06-20?? 13:15 +0800?Chen-Yu Tsai???
> Now that the device tree binding headers for the R_CCU have been
> merged,
> we can use the macros, instead of raw numbers.
> 
> Switch to R_CCU macros for clock and reset indices.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index c72da8cd9ef5..d85070f8c4a2 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -5,7 +5,9 @@
>  
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include <dt-bindings/clock/sun50i-h6-ccu.h>
> +#include <dt-bindings/clock/sun50i-h6-r-ccu.h>
>  #include <dt-bindings/reset/sun50i-h6-ccu.h>
> +#include <dt-bindings/reset/sun50i-h6-r-ccu.h>
>  
>  / {
>  	interrupt-parent = <&gic>;
> @@ -198,7 +200,7 @@
>  			reg = <0x07022000 0x400>;
>  			interrupts = <GIC_SPI 105
> IRQ_TYPE_LEVEL_HIGH>,
>  				     <GIC_SPI 111
> IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&r_ccu 2>, <&osc24M>, <&osc32k>;
> +			clocks = <&r_ccu CLK_R_APB1>, <&osc24M>,
> <&osc32k>;
>  			clock-names = "apb", "hosc", "losc";
>  			gpio-controller;
>  			#gpio-cells = <3>;
> @@ -208,6 +210,7 @@
>  			r_i2c_pins: r-i2c {
>  				pins = "PL0", "PL1";
>  				function = "s_i2c";
> +				bias-pull-up;

Should this be included in this patch?

>  			};
>  		};
>  
> @@ -215,8 +218,8 @@
>  			compatible = "allwinner,sun6i-a31-i2c";
>  			reg = <0x07081400 0x400>;
>  			interrupts = <GIC_SPI 107
> IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&r_ccu 8>;
> -			resets = <&r_ccu 4>;
> +			clocks = <&r_ccu CLK_R_APB2_I2C>;
> +			resets = <&r_ccu RST_R_APB2_I2C>;
>  			pinctrl-names = "default";
>  			pinctrl-0 = <&r_i2c_pins>;
>  			status = "disabled";

^ permalink raw reply

* [PATCH 1/6] dmaengine: xilinx_dma: fix splitting transfer causes misalignments
From: Andrea Merello @ 2018-06-20 13:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <DM6PR02MB4361458180616B591C6742E2C7770@DM6PR02MB4361.namprd02.prod.outlook.com>

On Wed, Jun 20, 2018 at 1:37 PM, Radhey Shyam Pandey <radheys@xilinx.com> wrote:
>> -----Original Message-----
>> From: dmaengine-owner at vger.kernel.org [mailto:dmaengine-
>> owner at vger.kernel.org] On Behalf Of Andrea Merello
>> Sent: Wednesday, June 20, 2018 2:07 PM
>> To: vkoul at kernel.org; dan.j.williams at intel.com; Michal Simek
>> <michals@xilinx.com>; Appana Durga Kedareswara Rao
>> <appanad@xilinx.com>; dmaengine at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
>> Andrea Merello <andrea.merello@gmail.com>
>> Subject: [PATCH 1/6] dmaengine: xilinx_dma: fix splitting transfer causes
>> misalignments
>
> We should rephrase commit message to something like "In axidma
> slave_sg and dma_cylic mode align split descriptors"

OK

>>
>> Whenever a single or cyclic transaction is prepared, the driver
>> could eventually split it over several SG descriptors in order
>> to deal with the HW maximum transfer length.
>>
>> This could end up in DMA operations starting from a misaligned
>> address. This seems fatal for the HW.
> This seems fatal for the HW if DRE is not enabled.

OK

>>
>> This patch eventually adjusts the transfer size in order to make sure
>> all operations start from an aligned address.
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> ---
>>  drivers/dma/xilinx/xilinx_dma.c | 27 ++++++++++++++++++++-------
>>  1 file changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index 27b523530c4a..a516e7ffef21 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -376,6 +376,7 @@ struct xilinx_dma_chan {
>>       void (*start_transfer)(struct xilinx_dma_chan *chan);
>>       int (*stop_transfer)(struct xilinx_dma_chan *chan);
>>       u16 tdest;
>> +     u32 copy_mask;
> We can reuse copy_align itself.  See below.

OK

>>  };
>>
>>  /**
>> @@ -1789,10 +1790,14 @@ static struct dma_async_tx_descriptor
>> *xilinx_dma_prep_slave_sg(
>>
>>                       /*
>>                        * Calculate the maximum number of bytes to transfer,
>> -                      * making sure it is less than the hw limit
>> +                      * making sure it is less than the hw limit and that
>> +                      * the next chuck start address is aligned
>
> /s/chuck/chunk

OK

>>                        */
>> -                     copy = min_t(size_t, sg_dma_len(sg) - sg_used,
>> -                                  XILINX_DMA_MAX_TRANS_LEN);
>> +                     copy = sg_dma_len(sg) - sg_used;
>> +                     if (copy > XILINX_DMA_MAX_TRANS_LEN)
>> +                             copy = XILINX_DMA_MAX_TRANS_LEN &
>> +                                     chan->copy_mask;
>> +
>
>
> In below implementation, we can reuse copy_align.
> Same for dma_cyclic.
>
> if ((copy + sg_used  < sg_dma_len(sg)) &&
>         chan->xdev->common.copy_align) {
>         /* If this is not the last descriptor, make sure
>           * the next one will be properly aligned
>          */
>         copy = rounddown(copy,
>                (1 << chan->xdev->common.copy_align));
>      }

OK for the general idea. But to me it seems a bit more complicated than needed:
What's the point in setting 'copy' with min_t, performing also the
subtraction sg_dma_len(sg) - sg_used, and then add sg_used again? What
about something like:


-                       copy = min_t(size_t, sg_dma_len(sg) - sg_used,
-                                    XILINX_DMA_MAX_TRANS_LEN);
+                       copy = sg_dma_len(sg) - sg_used;
+                       if (copy > XILINX_DMA_MAX_TRANS_LEN &&
+                           chan->xdev->common.copy_align)
+                               copy = rounddown(XILINX_DMA_MAX_TRANS_LEN,
+                                        (1 << chan->xdev->common.copy_align));
+


>>                       hw = &segment->hw;
>>
>>                       /* Fill in the descriptor */
>> @@ -1894,10 +1899,14 @@ static struct dma_async_tx_descriptor
>> *xilinx_dma_prep_dma_cyclic(
>>
>>                       /*
>>                        * Calculate the maximum number of bytes to transfer,
>> -                      * making sure it is less than the hw limit
>> +                      * making sure it is less than the hw limit and that
>> +                      * the next chuck start address is aligned
>>                        */
>> -                     copy = min_t(size_t, period_len - sg_used,
>> -                                  XILINX_DMA_MAX_TRANS_LEN);
>> +                     copy = period_len - sg_used;
>> +                     if (copy > XILINX_DMA_MAX_TRANS_LEN)
>> +                             copy = XILINX_DMA_MAX_TRANS_LEN &
>> +                                     chan->copy_mask;
>> +
>>                       hw = &segment->hw;
>>                       xilinx_axidma_buf(chan, hw, buf_addr, sg_used,
>>                                         period_len * i);
>> @@ -2402,8 +2411,12 @@ static int xilinx_dma_chan_probe(struct
>> xilinx_dma_device *xdev,
>>       if (width > 8)
>>               has_dre = false;
>>
>> -     if (!has_dre)
>> +     if (has_dre) {
>> +             chan->copy_mask = ~0;
>> +     } else {
>>               xdev->common.copy_align = fls(width - 1);
>> +             chan->copy_mask = ~(width - 1);
>> +     }
>
> As mentioned above we don't need this additional field.

OK

>>
>>       if (of_device_is_compatible(node, "xlnx,axi-vdma-mm2s-channel") ||
>>           of_device_is_compatible(node, "xlnx,axi-dma-mm2s-channel") ||
>> --
>> 2.17.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1
From: Ludovic BARRE @ 2018-06-20 13:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6162d701-2afd-5a22-d38f-7fb9e726cff7@roeck-us.net>



On 06/20/2018 11:19 AM, Guenter Roeck wrote:
> On 06/20/2018 12:53 AM, Ludovic Barre wrote:
>> From: Ludovic Barre <ludovic.barre@st.com>
>>
>> This patch adds config data to manage specific properties by
>> compatible. Adds stm32mp1 config which requires pclk clock.
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>> ---
>> ? .../devicetree/bindings/watchdog/st,stm32-iwdg.txt |? 21 +++-
>> ? drivers/watchdog/stm32_iwdg.c????????????????????? | 132 
>> ++++++++++++++-------
>> ? 2 files changed, 104 insertions(+), 49 deletions(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt 
>> b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>> index cc13b10a..f07f6d89 100644
>> --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>> +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>> @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG)
>> ? ---------------------------------
>> ? Required properties:
>> -- compatible: "st,stm32-iwdg"
>> -- reg: physical base address and length of the registers set for the 
>> device
>> -- clocks: must contain a single entry describing the clock input
>> +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg"
>> +- reg: Physical base address and length of the registers set for the 
>> device
>> +- clocks: Reference to the clock entry lsi. Additional pclk clock entry
>> +? is required only for st,stm32mp1-iwdg.
>> +- clock-names: Name of the clocks used.
>> +? "lsi" for st,stm32-iwdg
>> +? "pclk", "lsi" for st,stm32mp1-iwdg
>> ? Optional Properties:
>> ? - timeout-sec: Watchdog timeout value in seconds.
>> -Example:
>> +Examples:
>> ? iwdg: watchdog at 40003000 {
>> ????? compatible = "st,stm32-iwdg";
>> ????? reg = <0x40003000 0x400>;
>> ????? clocks = <&clk_lsi>;
>> +??? clock-names = "lsi";
>> +??? timeout-sec = <32>;
>> +};
>> +
>> +iwdg: iwdg at 5a002000 {
>> +??? compatible = "st,stm32mp1-iwdg";
>> +??? reg = <0x5a002000 0x400>;
>> +??? clocks = <&rcc IWDG2>, <&clk_lsi>;
>> +??? clock-names = "pclk", "lsi";
>> ????? timeout-sec = <32>;
>> ? };
>> diff --git a/drivers/watchdog/stm32_iwdg.c 
>> b/drivers/watchdog/stm32_iwdg.c
>> index c97ad56..fc96670 100644
>> --- a/drivers/watchdog/stm32_iwdg.c
>> +++ b/drivers/watchdog/stm32_iwdg.c
>> @@ -11,12 +11,13 @@
>> ? #include <linux/clk.h>
>> ? #include <linux/delay.h>
>> -#include <linux/kernel.h>
>> -#include <linux/module.h>
>> ? #include <linux/interrupt.h>
>> ? #include <linux/io.h>
>> ? #include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> ? #include <linux/of.h>
>> +#include <linux/of_device.h>
>> ? #include <linux/platform_device.h>
>> ? #include <linux/watchdog.h>
>> @@ -54,11 +55,17 @@
>> ? #define TIMEOUT_US??? 100000
>> ? #define SLEEP_US??? 1000
>> +struct stm32_iwdg_config {
>> +??? bool has_pclk;
>> +};
>> +
> 
> This data structure is unnecessary. Just assign the boolean directly to 
> .data
> and ...
> 

Ok, I send a v3, with boolean directly to .data
like:

#define NO_PCLK		false
#define HAS_PCLK	true
...
static const struct of_device_id stm32_iwdg_of_match[] = {
	{ .compatible = "st,stm32-iwdg", .data = (void *) NO_PCLK },
	{ .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK },
	{ /* end node */ }
};

Note:
V3, because I sent my original version with V2
(it's mistake)

>> ? struct stm32_iwdg {
>> -??? struct watchdog_device??? wdd;
>> -??? void __iomem??????? *regs;
>> -??? struct clk??????? *clk;
>> -??? unsigned int??????? rate;
>> +??? struct watchdog_device??????? wdd;
>> +??? void __iomem??????????? *regs;
>> +??? struct stm32_iwdg_config??? *config;
> 
> declare bool has_pclk here.
> 
>> +??? struct clk??????????? *clk_lsi;
>> +??? struct clk??????????? *clk_pclk;
>> +??? unsigned int??????????? rate;
>> ? };
>> ? static inline u32 reg_read(void __iomem *base, u32 reg)
>> @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct 
>> watchdog_device *wdd,
>> ????? return 0;
>> ? }
>> +static int stm32_iwdg_clk_init(struct platform_device *pdev,
>> +?????????????????? struct stm32_iwdg *wdt)
>> +{
>> +??? u32 ret;
>> +
>> +??? wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi");
>> +??? if (IS_ERR(wdt->clk_lsi)) {
>> +??????? dev_err(&pdev->dev, "Unable to get lsi clock\n");
>> +??????? return PTR_ERR(wdt->clk_lsi);
>> +??? }
>> +
>> +??? /* optional peripheral clock */
>> +??? if (wdt->config->has_pclk) {
>> +??????? wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk");
>> +??????? if (IS_ERR(wdt->clk_pclk)) {
>> +??????????? dev_err(&pdev->dev, "Unable to get pclk clock\n");
>> +??????????? return PTR_ERR(wdt->clk_pclk);
>> +??????? }
>> +
>> +??????? ret = clk_prepare_enable(wdt->clk_pclk);
>> +??????? if (ret) {
>> +??????????? dev_err(&pdev->dev, "Unable to prepare pclk clock\n");
>> +??????????? return ret;
>> +??????? }
>> +??? }
>> +
>> +??? ret = clk_prepare_enable(wdt->clk_lsi);
>> +??? if (ret) {
>> +??????? dev_err(&pdev->dev, "Unable to prepare lsi clock\n");
>> +??????? clk_disable_unprepare(wdt->clk_pclk);
>> +??????? return ret;
>> +??? }
>> +
>> +??? wdt->rate = clk_get_rate(wdt->clk_lsi);
>> +
>> +??? return 0;
>> +}
>> +
>> ? static const struct watchdog_info stm32_iwdg_info = {
>> ????? .options??? = WDIOF_SETTIMEOUT |
>> ??????????????? WDIOF_MAGICCLOSE |
>> @@ -147,49 +192,50 @@ static const struct watchdog_ops stm32_iwdg_ops = {
>> ????? .set_timeout??? = stm32_iwdg_set_timeout,
>> ? };
>> +static const struct stm32_iwdg_config stm32_iwdg_cfg = {
>> +??? .has_pclk = false,
>> +};
>> +
>> +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = {
>> +??? .has_pclk = true,
>> +};
>> +
>> +static const struct of_device_id stm32_iwdg_of_match[] = {
>> +??? { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg },
>> +??? { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg },
>> +??? { /* end node */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
>> +
>> ? static int stm32_iwdg_probe(struct platform_device *pdev)
>> ? {
>> ????? struct watchdog_device *wdd;
>> +??? const struct of_device_id *match;
>> ????? struct stm32_iwdg *wdt;
>> ????? struct resource *res;
>> -??? void __iomem *regs;
>> -??? struct clk *clk;
>> ????? int ret;
>> +??? match = of_match_device(stm32_iwdg_of_match, &pdev->dev);
>> +??? if (!match || !match->data)
>> +??????? return -ENODEV;
>> +
>> +??? wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>> +??? if (!wdt)
>> +??????? return -ENOMEM;
>> +
>> +??? wdt->config = (struct stm32_iwdg_config *)match->data;
>> +
>> ????? /* This is the timer base. */
>> ????? res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -??? regs = devm_ioremap_resource(&pdev->dev, res);
>> -??? if (IS_ERR(regs)) {
>> +??? wdt->regs = devm_ioremap_resource(&pdev->dev, res);
>> +??? if (IS_ERR(wdt->regs)) {
>> ????????? dev_err(&pdev->dev, "Could not get resource\n");
>> -??????? return PTR_ERR(regs);
>> +??????? return PTR_ERR(wdt->regs);
>> ????? }
>> -??? clk = devm_clk_get(&pdev->dev, NULL);
>> -??? if (IS_ERR(clk)) {
>> -??????? dev_err(&pdev->dev, "Unable to get clock\n");
>> -??????? return PTR_ERR(clk);
>> -??? }
>> -
>> -??? ret = clk_prepare_enable(clk);
>> -??? if (ret) {
>> -??????? dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk);
>> +??? ret = stm32_iwdg_clk_init(pdev, wdt);
>> +??? if (ret)
>> ????????? return ret;
>> -??? }
>> -
>> -??? /*
>> -???? * Allocate our watchdog driver data, which has the
>> -???? * struct watchdog_device nested within it.
>> -???? */
>> -??? wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>> -??? if (!wdt) {
>> -??????? ret = -ENOMEM;
>> -??????? goto err;
>> -??? }
>> -
>> -??? /* Initialize struct stm32_iwdg. */
>> -??? wdt->regs = regs;
>> -??? wdt->clk = clk;
>> -??? wdt->rate = clk_get_rate(clk);
>> ????? /* Initialize struct watchdog_device. */
>> ????? wdd = &wdt->wdd;
>> @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct platform_device 
>> *pdev)
>> ????? return 0;
>> ? err:
>> -??? clk_disable_unprepare(clk);
>> +??? clk_disable_unprepare(wdt->clk_lsi);
>> +??? clk_disable_unprepare(wdt->clk_pclk);
>> ????? return ret;
>> ? }
>> @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct 
>> platform_device *pdev)
>> ????? struct stm32_iwdg *wdt = platform_get_drvdata(pdev);
>> ????? watchdog_unregister_device(&wdt->wdd);
>> -??? clk_disable_unprepare(wdt->clk);
>> +??? clk_disable_unprepare(wdt->clk_lsi);
>> +??? clk_disable_unprepare(wdt->clk_pclk);
>> ????? return 0;
>> ? }
>> -static const struct of_device_id stm32_iwdg_of_match[] = {
>> -??? { .compatible = "st,stm32-iwdg" },
>> -??? { /* end node */ }
>> -};
>> -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
>> -
>> ? static struct platform_driver stm32_iwdg_driver = {
>> ????? .probe??????? = stm32_iwdg_probe,
>> ????? .remove??????? = stm32_iwdg_remove,
>> ????? .driver = {
>> ????????? .name??? = "iwdg",
>> -??????? .of_match_table = stm32_iwdg_of_match,
>> +??????? .of_match_table = of_match_ptr(stm32_iwdg_of_match),
>> ????? },
>> ? };
>> ? module_platform_driver(stm32_iwdg_driver);
>>
> 

^ permalink raw reply

* [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1
From: Guenter Roeck @ 2018-06-20 13:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <e9babb86-7588-5de6-93ab-4cdd8982e773@st.com>

On 06/20/2018 06:24 AM, Ludovic BARRE wrote:
> 
> 
> On 06/20/2018 11:19 AM, Guenter Roeck wrote:
>> On 06/20/2018 12:53 AM, Ludovic Barre wrote:
>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>
>>> This patch adds config data to manage specific properties by
>>> compatible. Adds stm32mp1 config which requires pclk clock.
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>> ---
>>> ? .../devicetree/bindings/watchdog/st,stm32-iwdg.txt |? 21 +++-
>>> ? drivers/watchdog/stm32_iwdg.c????????????????????? | 132 ++++++++++++++-------
>>> ? 2 files changed, 104 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>>> index cc13b10a..f07f6d89 100644
>>> --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>>> +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>>> @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG)
>>> ? ---------------------------------
>>> ? Required properties:
>>> -- compatible: "st,stm32-iwdg"
>>> -- reg: physical base address and length of the registers set for the device
>>> -- clocks: must contain a single entry describing the clock input
>>> +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg"
>>> +- reg: Physical base address and length of the registers set for the device
>>> +- clocks: Reference to the clock entry lsi. Additional pclk clock entry
>>> +? is required only for st,stm32mp1-iwdg.
>>> +- clock-names: Name of the clocks used.
>>> +? "lsi" for st,stm32-iwdg
>>> +? "pclk", "lsi" for st,stm32mp1-iwdg
>>> ? Optional Properties:
>>> ? - timeout-sec: Watchdog timeout value in seconds.
>>> -Example:
>>> +Examples:
>>> ? iwdg: watchdog at 40003000 {
>>> ????? compatible = "st,stm32-iwdg";
>>> ????? reg = <0x40003000 0x400>;
>>> ????? clocks = <&clk_lsi>;
>>> +??? clock-names = "lsi";
>>> +??? timeout-sec = <32>;
>>> +};
>>> +
>>> +iwdg: iwdg at 5a002000 {
>>> +??? compatible = "st,stm32mp1-iwdg";
>>> +??? reg = <0x5a002000 0x400>;
>>> +??? clocks = <&rcc IWDG2>, <&clk_lsi>;
>>> +??? clock-names = "pclk", "lsi";
>>> ????? timeout-sec = <32>;
>>> ? };
>>> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
>>> index c97ad56..fc96670 100644
>>> --- a/drivers/watchdog/stm32_iwdg.c
>>> +++ b/drivers/watchdog/stm32_iwdg.c
>>> @@ -11,12 +11,13 @@
>>> ? #include <linux/clk.h>
>>> ? #include <linux/delay.h>
>>> -#include <linux/kernel.h>
>>> -#include <linux/module.h>
>>> ? #include <linux/interrupt.h>
>>> ? #include <linux/io.h>
>>> ? #include <linux/iopoll.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> ? #include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> ? #include <linux/platform_device.h>
>>> ? #include <linux/watchdog.h>
>>> @@ -54,11 +55,17 @@
>>> ? #define TIMEOUT_US??? 100000
>>> ? #define SLEEP_US??? 1000
>>> +struct stm32_iwdg_config {
>>> +??? bool has_pclk;
>>> +};
>>> +
>>
>> This data structure is unnecessary. Just assign the boolean directly to .data
>> and ...
>>
> 
> Ok, I send a v3, with boolean directly to .data
> like:
> 
> #define NO_PCLK??????? false
> #define HAS_PCLK??? true
> ...

Just use true/false directly. There is no need for those defines.

If you want the reader to understand what is defined, I would be ok with

#define HAS_PCLK    true

static const struct of_device_id stm32_iwdg_of_match[] = {
      { .compatible = "st,stm32-iwdg", .data = (void *) !HAS_PCLK },
      { .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK },
      { /* end node */ }

Guenter

> static const struct of_device_id stm32_iwdg_of_match[] = {
>  ????{ .compatible = "st,stm32-iwdg", .data = (void *) NO_PCLK },
>  ????{ .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK },
>  ????{ /* end node */ }
> };
> 
> Note:
> V3, because I sent my original version with V2
> (it's mistake)
> 
>>> ? struct stm32_iwdg {
>>> -??? struct watchdog_device??? wdd;
>>> -??? void __iomem??????? *regs;
>>> -??? struct clk??????? *clk;
>>> -??? unsigned int??????? rate;
>>> +??? struct watchdog_device??????? wdd;
>>> +??? void __iomem??????????? *regs;
>>> +??? struct stm32_iwdg_config??? *config;
>>
>> declare bool has_pclk here.
>>
>>> +??? struct clk??????????? *clk_lsi;
>>> +??? struct clk??????????? *clk_pclk;
>>> +??? unsigned int??????????? rate;
>>> ? };
>>> ? static inline u32 reg_read(void __iomem *base, u32 reg)
>>> @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
>>> ????? return 0;
>>> ? }
>>> +static int stm32_iwdg_clk_init(struct platform_device *pdev,
>>> +?????????????????? struct stm32_iwdg *wdt)
>>> +{
>>> +??? u32 ret;
>>> +
>>> +??? wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi");
>>> +??? if (IS_ERR(wdt->clk_lsi)) {
>>> +??????? dev_err(&pdev->dev, "Unable to get lsi clock\n");
>>> +??????? return PTR_ERR(wdt->clk_lsi);
>>> +??? }
>>> +
>>> +??? /* optional peripheral clock */
>>> +??? if (wdt->config->has_pclk) {
>>> +??????? wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk");
>>> +??????? if (IS_ERR(wdt->clk_pclk)) {
>>> +??????????? dev_err(&pdev->dev, "Unable to get pclk clock\n");
>>> +??????????? return PTR_ERR(wdt->clk_pclk);
>>> +??????? }
>>> +
>>> +??????? ret = clk_prepare_enable(wdt->clk_pclk);
>>> +??????? if (ret) {
>>> +??????????? dev_err(&pdev->dev, "Unable to prepare pclk clock\n");
>>> +??????????? return ret;
>>> +??????? }
>>> +??? }
>>> +
>>> +??? ret = clk_prepare_enable(wdt->clk_lsi);
>>> +??? if (ret) {
>>> +??????? dev_err(&pdev->dev, "Unable to prepare lsi clock\n");
>>> +??????? clk_disable_unprepare(wdt->clk_pclk);
>>> +??????? return ret;
>>> +??? }
>>> +
>>> +??? wdt->rate = clk_get_rate(wdt->clk_lsi);
>>> +
>>> +??? return 0;
>>> +}
>>> +
>>> ? static const struct watchdog_info stm32_iwdg_info = {
>>> ????? .options??? = WDIOF_SETTIMEOUT |
>>> ??????????????? WDIOF_MAGICCLOSE |
>>> @@ -147,49 +192,50 @@ static const struct watchdog_ops stm32_iwdg_ops = {
>>> ????? .set_timeout??? = stm32_iwdg_set_timeout,
>>> ? };
>>> +static const struct stm32_iwdg_config stm32_iwdg_cfg = {
>>> +??? .has_pclk = false,
>>> +};
>>> +
>>> +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = {
>>> +??? .has_pclk = true,
>>> +};
>>> +
>>> +static const struct of_device_id stm32_iwdg_of_match[] = {
>>> +??? { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg },
>>> +??? { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg },
>>> +??? { /* end node */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
>>> +
>>> ? static int stm32_iwdg_probe(struct platform_device *pdev)
>>> ? {
>>> ????? struct watchdog_device *wdd;
>>> +??? const struct of_device_id *match;
>>> ????? struct stm32_iwdg *wdt;
>>> ????? struct resource *res;
>>> -??? void __iomem *regs;
>>> -??? struct clk *clk;
>>> ????? int ret;
>>> +??? match = of_match_device(stm32_iwdg_of_match, &pdev->dev);
>>> +??? if (!match || !match->data)
>>> +??????? return -ENODEV;
>>> +
>>> +??? wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>>> +??? if (!wdt)
>>> +??????? return -ENOMEM;
>>> +
>>> +??? wdt->config = (struct stm32_iwdg_config *)match->data;
>>> +
>>> ????? /* This is the timer base. */
>>> ????? res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> -??? regs = devm_ioremap_resource(&pdev->dev, res);
>>> -??? if (IS_ERR(regs)) {
>>> +??? wdt->regs = devm_ioremap_resource(&pdev->dev, res);
>>> +??? if (IS_ERR(wdt->regs)) {
>>> ????????? dev_err(&pdev->dev, "Could not get resource\n");
>>> -??????? return PTR_ERR(regs);
>>> +??????? return PTR_ERR(wdt->regs);
>>> ????? }
>>> -??? clk = devm_clk_get(&pdev->dev, NULL);
>>> -??? if (IS_ERR(clk)) {
>>> -??????? dev_err(&pdev->dev, "Unable to get clock\n");
>>> -??????? return PTR_ERR(clk);
>>> -??? }
>>> -
>>> -??? ret = clk_prepare_enable(clk);
>>> -??? if (ret) {
>>> -??????? dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk);
>>> +??? ret = stm32_iwdg_clk_init(pdev, wdt);
>>> +??? if (ret)
>>> ????????? return ret;
>>> -??? }
>>> -
>>> -??? /*
>>> -???? * Allocate our watchdog driver data, which has the
>>> -???? * struct watchdog_device nested within it.
>>> -???? */
>>> -??? wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>>> -??? if (!wdt) {
>>> -??????? ret = -ENOMEM;
>>> -??????? goto err;
>>> -??? }
>>> -
>>> -??? /* Initialize struct stm32_iwdg. */
>>> -??? wdt->regs = regs;
>>> -??? wdt->clk = clk;
>>> -??? wdt->rate = clk_get_rate(clk);
>>> ????? /* Initialize struct watchdog_device. */
>>> ????? wdd = &wdt->wdd;
>>> @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct platform_device *pdev)
>>> ????? return 0;
>>> ? err:
>>> -??? clk_disable_unprepare(clk);
>>> +??? clk_disable_unprepare(wdt->clk_lsi);
>>> +??? clk_disable_unprepare(wdt->clk_pclk);
>>> ????? return ret;
>>> ? }
>>> @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct platform_device *pdev)
>>> ????? struct stm32_iwdg *wdt = platform_get_drvdata(pdev);
>>> ????? watchdog_unregister_device(&wdt->wdd);
>>> -??? clk_disable_unprepare(wdt->clk);
>>> +??? clk_disable_unprepare(wdt->clk_lsi);
>>> +??? clk_disable_unprepare(wdt->clk_pclk);
>>> ????? return 0;
>>> ? }
>>> -static const struct of_device_id stm32_iwdg_of_match[] = {
>>> -??? { .compatible = "st,stm32-iwdg" },
>>> -??? { /* end node */ }
>>> -};
>>> -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
>>> -
>>> ? static struct platform_driver stm32_iwdg_driver = {
>>> ????? .probe??????? = stm32_iwdg_probe,
>>> ????? .remove??????? = stm32_iwdg_remove,
>>> ????? .driver = {
>>> ????????? .name??? = "iwdg",
>>> -??????? .of_match_table = stm32_iwdg_of_match,
>>> +??????? .of_match_table = of_match_ptr(stm32_iwdg_of_match),
>>> ????? },
>>> ? };
>>> ? module_platform_driver(stm32_iwdg_driver);
>>>
>>
> 

^ permalink raw reply

* [PATCH 2/6] dmaengine: xilinx_dma: fix completion callback is not invoked for each DMA operation
From: Andrea Merello @ 2018-06-20 13:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <DM6PR02MB43615ED0221189B6B8D9017EC7770@DM6PR02MB4361.namprd02.prod.outlook.com>

On Wed, Jun 20, 2018 at 2:36 PM, Radhey Shyam Pandey <radheys@xilinx.com> wrote:
>> -----Original Message-----
>> From: dmaengine-owner at vger.kernel.org [mailto:dmaengine-
>> owner at vger.kernel.org] On Behalf Of Andrea Merello
>> Sent: Wednesday, June 20, 2018 2:07 PM
>> To: vkoul at kernel.org; dan.j.williams at intel.com; Michal Simek
>> <michals@xilinx.com>; Appana Durga Kedareswara Rao
>> <appanad@xilinx.com>; dmaengine at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
>> Andrea Merello <andrea.merello@gmail.com>
>> Subject: [PATCH 2/6] dmaengine: xilinx_dma: fix completion callback is not
>> invoked for each DMA operation
>>
>> API specification says: "On completion of each DMA operation, the next in
>> queue is started and a tasklet triggered. The tasklet will then call the
>> client driver completion callback routine for notification, if set."
>>
>> Currently the driver keeps a "desc_pendingcount" counter of the total
>> descriptor pending, and it uses as IRQ coalesce threshold, as result it
>> only calls the CBs after ALL pending operations are completed, which is
>> wrong.
> I think IRQ coalescing enable/disable should be configurable.
> Performance related usecases will need this support.

I didn't intend this (only) wrt performances; my concern was mostly
wrt correctness. If my point of view is wrong then I'll drop this
patch from the series.

(.. I might respin it again in future: I had a patch wrt an old driver
version that allowed submitting new descriptors to the HW while the
DMA is running, and in this case disabling coalesce is needed i.e. in
order to submit a new empty buffer whenever the DMA finishes a
transfer without waiting the DMA to stop).

BTW, is there any dmaengine API suitable for setting interrupt coalesce?

>>
>> This patch uses disable IRQ coalesce and checks for the completion flag
>> for the descriptors (which is further divided in segments).
>>
>> Possibly a better optimization could be using proper IRQ coalesce
>> threshold to get an IRQ after all segments of the descriptors are done.
>> But we don't do that yet..
>>
>> NOTE: for now we do this only for AXI DMA, other DMA flavors are
>> untested/untouched.
>> This is loosely based on
>> commit 65df81a6dc74 ("xilinx_dma: IrqThreshold set incorrectly, unreliable.")
>> in my linux-4.6-zynq tree
> NOTE description doesn't help much.
>
>>
>> From: Jeremy Trimble [original patch]
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> ---
>>  drivers/dma/xilinx/xilinx_dma.c | 39 +++++++++++++++++++++------------
>>  1 file changed, 25 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index a516e7ffef21..cf12f7147f07 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -164,6 +164,7 @@
>>  #define XILINX_DMA_CR_COALESCE_SHIFT 16
>>  #define XILINX_DMA_BD_SOP            BIT(27)
>>  #define XILINX_DMA_BD_EOP            BIT(26)
>> +#define XILINX_DMA_BD_CMPLT          BIT(31)
>>  #define XILINX_DMA_COALESCE_MAX              255
>>  #define XILINX_DMA_NUM_DESCS         255
>>  #define XILINX_DMA_NUM_APP_WORDS     5
>> @@ -1274,12 +1275,9 @@ static void xilinx_dma_start_transfer(struct
>> xilinx_dma_chan *chan)
>>
>>       reg = dma_ctrl_read(chan, XILINX_DMA_REG_DMACR);
>>
>> -     if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) {
>> -             reg &= ~XILINX_DMA_CR_COALESCE_MAX;
>> -             reg |= chan->desc_pendingcount <<
>> -                               XILINX_DMA_CR_COALESCE_SHIFT;
>> -             dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>> -     }
>> +     reg &= ~XILINX_DMA_CR_COALESCE_MAX;
>> +     reg |= 1 << XILINX_DMA_CR_COALESCE_SHIFT;
>> +     dma_ctrl_write(chan, XILINX_DMA_REG_DMACR, reg);
>>
>>       if (chan->has_sg && !chan->xdev->mcdma)
>>               xilinx_write(chan, XILINX_DMA_REG_CURDESC,
>> @@ -1378,6 +1376,20 @@ static void xilinx_dma_complete_descriptor(struct
>> xilinx_dma_chan *chan)
>>               return;
>>
>>       list_for_each_entry_safe(desc, next, &chan->active_list, node) {
>> +             if (chan->xdev->dma_config->dmatype ==
>> XDMA_TYPE_AXIDMA) {
>> +                     /*
>> +                      * Check whether the last segment in this descriptor
>> +                      * has been completed.
>> +                      */
>> +                     const struct xilinx_axidma_tx_segment *const tail_seg
>> =
>> +                             list_last_entry(&desc->segments,
>> +                                             struct
>> xilinx_axidma_tx_segment,
>> +                                             node);
>> +
>> +                     /* we've processed all the completed descriptors */
>> +                     if (!(tail_seg->hw.status & XILINX_DMA_BD_CMPLT))
>> +                             break;
>> +             }
>>               list_del(&desc->node);
>>               if (!desc->cyclic)
>>                       dma_cookie_complete(&desc->async_tx);
>> @@ -1826,14 +1838,13 @@ static struct dma_async_tx_descriptor
>> *xilinx_dma_prep_slave_sg(
>>                                  struct xilinx_axidma_tx_segment, node);
>>       desc->async_tx.phys = segment->phys;
>>
>> -     /* For the last DMA_MEM_TO_DEV transfer, set EOP */
>> -     if (chan->direction == DMA_MEM_TO_DEV) {
>> -             segment->hw.control |= XILINX_DMA_BD_SOP;
>> -             segment = list_last_entry(&desc->segments,
>> -                                       struct xilinx_axidma_tx_segment,
>> -                                       node);
>> -             segment->hw.control |= XILINX_DMA_BD_EOP;
>> -     }
>> +     /* For the first transfer, set SOP */
>> +     segment->hw.control |= XILINX_DMA_BD_SOP;
>> +     /* For the last transfer, set EOP */
>> +     segment = list_last_entry(&desc->segments,
>> +                               struct xilinx_axidma_tx_segment,
>> +                               node);
>> +     segment->hw.control |= XILINX_DMA_BD_EOP;
>>
>>       return &desc->async_tx;
>>
>> --
>> 2.17.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1
From: Ludovic BARRE @ 2018-06-20 13:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <d1d5e5f8-2e0f-7a73-ec43-6bb1f3d5f19c@roeck-us.net>



On 06/20/2018 03:29 PM, Guenter Roeck wrote:
> On 06/20/2018 06:24 AM, Ludovic BARRE wrote:
>>
>>
>> On 06/20/2018 11:19 AM, Guenter Roeck wrote:
>>> On 06/20/2018 12:53 AM, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> This patch adds config data to manage specific properties by
>>>> compatible. Adds stm32mp1 config which requires pclk clock.
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>> ? .../devicetree/bindings/watchdog/st,stm32-iwdg.txt |? 21 +++-
>>>> ? drivers/watchdog/stm32_iwdg.c????????????????????? | 132 
>>>> ++++++++++++++-------
>>>> ? 2 files changed, 104 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt 
>>>> b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>>>> index cc13b10a..f07f6d89 100644
>>>> --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>>>> +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
>>>> @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG)
>>>> ? ---------------------------------
>>>> ? Required properties:
>>>> -- compatible: "st,stm32-iwdg"
>>>> -- reg: physical base address and length of the registers set for 
>>>> the device
>>>> -- clocks: must contain a single entry describing the clock input
>>>> +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg"
>>>> +- reg: Physical base address and length of the registers set for 
>>>> the device
>>>> +- clocks: Reference to the clock entry lsi. Additional pclk clock 
>>>> entry
>>>> +? is required only for st,stm32mp1-iwdg.
>>>> +- clock-names: Name of the clocks used.
>>>> +? "lsi" for st,stm32-iwdg
>>>> +? "pclk", "lsi" for st,stm32mp1-iwdg
>>>> ? Optional Properties:
>>>> ? - timeout-sec: Watchdog timeout value in seconds.
>>>> -Example:
>>>> +Examples:
>>>> ? iwdg: watchdog at 40003000 {
>>>> ????? compatible = "st,stm32-iwdg";
>>>> ????? reg = <0x40003000 0x400>;
>>>> ????? clocks = <&clk_lsi>;
>>>> +??? clock-names = "lsi";
>>>> +??? timeout-sec = <32>;
>>>> +};
>>>> +
>>>> +iwdg: iwdg at 5a002000 {
>>>> +??? compatible = "st,stm32mp1-iwdg";
>>>> +??? reg = <0x5a002000 0x400>;
>>>> +??? clocks = <&rcc IWDG2>, <&clk_lsi>;
>>>> +??? clock-names = "pclk", "lsi";
>>>> ????? timeout-sec = <32>;
>>>> ? };
>>>> diff --git a/drivers/watchdog/stm32_iwdg.c 
>>>> b/drivers/watchdog/stm32_iwdg.c
>>>> index c97ad56..fc96670 100644
>>>> --- a/drivers/watchdog/stm32_iwdg.c
>>>> +++ b/drivers/watchdog/stm32_iwdg.c
>>>> @@ -11,12 +11,13 @@
>>>> ? #include <linux/clk.h>
>>>> ? #include <linux/delay.h>
>>>> -#include <linux/kernel.h>
>>>> -#include <linux/module.h>
>>>> ? #include <linux/interrupt.h>
>>>> ? #include <linux/io.h>
>>>> ? #include <linux/iopoll.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/module.h>
>>>> ? #include <linux/of.h>
>>>> +#include <linux/of_device.h>
>>>> ? #include <linux/platform_device.h>
>>>> ? #include <linux/watchdog.h>
>>>> @@ -54,11 +55,17 @@
>>>> ? #define TIMEOUT_US??? 100000
>>>> ? #define SLEEP_US??? 1000
>>>> +struct stm32_iwdg_config {
>>>> +??? bool has_pclk;
>>>> +};
>>>> +
>>>
>>> This data structure is unnecessary. Just assign the boolean directly 
>>> to .data
>>> and ...
>>>
>>
>> Ok, I send a v3, with boolean directly to .data
>> like:
>>
>> #define NO_PCLK??????? false
>> #define HAS_PCLK??? true
>> ...
> 
> Just use true/false directly. There is no need for those defines.
> 
> If you want the reader to understand what is defined, I would be ok with
> 
> #define HAS_PCLK??? true
> 
> static const struct of_device_id stm32_iwdg_of_match[] = {
>  ???? { .compatible = "st,stm32-iwdg", .data = (void *) !HAS_PCLK },
>  ???? { .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK },
>  ???? { /* end node */ }
> 
> Guenter

Ok, thanks

> 
>> static const struct of_device_id stm32_iwdg_of_match[] = {
>> ?????{ .compatible = "st,stm32-iwdg", .data = (void *) NO_PCLK },
>> ?????{ .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK },
>> ?????{ /* end node */ }
>> };
>>
>> Note:
>> V3, because I sent my original version with V2
>> (it's mistake)
>>
>>>> ? struct stm32_iwdg {
>>>> -??? struct watchdog_device??? wdd;
>>>> -??? void __iomem??????? *regs;
>>>> -??? struct clk??????? *clk;
>>>> -??? unsigned int??????? rate;
>>>> +??? struct watchdog_device??????? wdd;
>>>> +??? void __iomem??????????? *regs;
>>>> +??? struct stm32_iwdg_config??? *config;
>>>
>>> declare bool has_pclk here.
>>>
>>>> +??? struct clk??????????? *clk_lsi;
>>>> +??? struct clk??????????? *clk_pclk;
>>>> +??? unsigned int??????????? rate;
>>>> ? };
>>>> ? static inline u32 reg_read(void __iomem *base, u32 reg)
>>>> @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct 
>>>> watchdog_device *wdd,
>>>> ????? return 0;
>>>> ? }
>>>> +static int stm32_iwdg_clk_init(struct platform_device *pdev,
>>>> +?????????????????? struct stm32_iwdg *wdt)
>>>> +{
>>>> +??? u32 ret;
>>>> +
>>>> +??? wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi");
>>>> +??? if (IS_ERR(wdt->clk_lsi)) {
>>>> +??????? dev_err(&pdev->dev, "Unable to get lsi clock\n");
>>>> +??????? return PTR_ERR(wdt->clk_lsi);
>>>> +??? }
>>>> +
>>>> +??? /* optional peripheral clock */
>>>> +??? if (wdt->config->has_pclk) {
>>>> +??????? wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk");
>>>> +??????? if (IS_ERR(wdt->clk_pclk)) {
>>>> +??????????? dev_err(&pdev->dev, "Unable to get pclk clock\n");
>>>> +??????????? return PTR_ERR(wdt->clk_pclk);
>>>> +??????? }
>>>> +
>>>> +??????? ret = clk_prepare_enable(wdt->clk_pclk);
>>>> +??????? if (ret) {
>>>> +??????????? dev_err(&pdev->dev, "Unable to prepare pclk clock\n");
>>>> +??????????? return ret;
>>>> +??????? }
>>>> +??? }
>>>> +
>>>> +??? ret = clk_prepare_enable(wdt->clk_lsi);
>>>> +??? if (ret) {
>>>> +??????? dev_err(&pdev->dev, "Unable to prepare lsi clock\n");
>>>> +??????? clk_disable_unprepare(wdt->clk_pclk);
>>>> +??????? return ret;
>>>> +??? }
>>>> +
>>>> +??? wdt->rate = clk_get_rate(wdt->clk_lsi);
>>>> +
>>>> +??? return 0;
>>>> +}
>>>> +
>>>> ? static const struct watchdog_info stm32_iwdg_info = {
>>>> ????? .options??? = WDIOF_SETTIMEOUT |
>>>> ??????????????? WDIOF_MAGICCLOSE |
>>>> @@ -147,49 +192,50 @@ static const struct watchdog_ops 
>>>> stm32_iwdg_ops = {
>>>> ????? .set_timeout??? = stm32_iwdg_set_timeout,
>>>> ? };
>>>> +static const struct stm32_iwdg_config stm32_iwdg_cfg = {
>>>> +??? .has_pclk = false,
>>>> +};
>>>> +
>>>> +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = {
>>>> +??? .has_pclk = true,
>>>> +};
>>>> +
>>>> +static const struct of_device_id stm32_iwdg_of_match[] = {
>>>> +??? { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg },
>>>> +??? { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg },
>>>> +??? { /* end node */ }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
>>>> +
>>>> ? static int stm32_iwdg_probe(struct platform_device *pdev)
>>>> ? {
>>>> ????? struct watchdog_device *wdd;
>>>> +??? const struct of_device_id *match;
>>>> ????? struct stm32_iwdg *wdt;
>>>> ????? struct resource *res;
>>>> -??? void __iomem *regs;
>>>> -??? struct clk *clk;
>>>> ????? int ret;
>>>> +??? match = of_match_device(stm32_iwdg_of_match, &pdev->dev);
>>>> +??? if (!match || !match->data)
>>>> +??????? return -ENODEV;
>>>> +
>>>> +??? wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>>>> +??? if (!wdt)
>>>> +??????? return -ENOMEM;
>>>> +
>>>> +??? wdt->config = (struct stm32_iwdg_config *)match->data;
>>>> +
>>>> ????? /* This is the timer base. */
>>>> ????? res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>> -??? regs = devm_ioremap_resource(&pdev->dev, res);
>>>> -??? if (IS_ERR(regs)) {
>>>> +??? wdt->regs = devm_ioremap_resource(&pdev->dev, res);
>>>> +??? if (IS_ERR(wdt->regs)) {
>>>> ????????? dev_err(&pdev->dev, "Could not get resource\n");
>>>> -??????? return PTR_ERR(regs);
>>>> +??????? return PTR_ERR(wdt->regs);
>>>> ????? }
>>>> -??? clk = devm_clk_get(&pdev->dev, NULL);
>>>> -??? if (IS_ERR(clk)) {
>>>> -??????? dev_err(&pdev->dev, "Unable to get clock\n");
>>>> -??????? return PTR_ERR(clk);
>>>> -??? }
>>>> -
>>>> -??? ret = clk_prepare_enable(clk);
>>>> -??? if (ret) {
>>>> -??????? dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk);
>>>> +??? ret = stm32_iwdg_clk_init(pdev, wdt);
>>>> +??? if (ret)
>>>> ????????? return ret;
>>>> -??? }
>>>> -
>>>> -??? /*
>>>> -???? * Allocate our watchdog driver data, which has the
>>>> -???? * struct watchdog_device nested within it.
>>>> -???? */
>>>> -??? wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
>>>> -??? if (!wdt) {
>>>> -??????? ret = -ENOMEM;
>>>> -??????? goto err;
>>>> -??? }
>>>> -
>>>> -??? /* Initialize struct stm32_iwdg. */
>>>> -??? wdt->regs = regs;
>>>> -??? wdt->clk = clk;
>>>> -??? wdt->rate = clk_get_rate(clk);
>>>> ????? /* Initialize struct watchdog_device. */
>>>> ????? wdd = &wdt->wdd;
>>>> @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct 
>>>> platform_device *pdev)
>>>> ????? return 0;
>>>> ? err:
>>>> -??? clk_disable_unprepare(clk);
>>>> +??? clk_disable_unprepare(wdt->clk_lsi);
>>>> +??? clk_disable_unprepare(wdt->clk_pclk);
>>>> ????? return ret;
>>>> ? }
>>>> @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct 
>>>> platform_device *pdev)
>>>> ????? struct stm32_iwdg *wdt = platform_get_drvdata(pdev);
>>>> ????? watchdog_unregister_device(&wdt->wdd);
>>>> -??? clk_disable_unprepare(wdt->clk);
>>>> +??? clk_disable_unprepare(wdt->clk_lsi);
>>>> +??? clk_disable_unprepare(wdt->clk_pclk);
>>>> ????? return 0;
>>>> ? }
>>>> -static const struct of_device_id stm32_iwdg_of_match[] = {
>>>> -??? { .compatible = "st,stm32-iwdg" },
>>>> -??? { /* end node */ }
>>>> -};
>>>> -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
>>>> -
>>>> ? static struct platform_driver stm32_iwdg_driver = {
>>>> ????? .probe??????? = stm32_iwdg_probe,
>>>> ????? .remove??????? = stm32_iwdg_remove,
>>>> ????? .driver = {
>>>> ????????? .name??? = "iwdg",
>>>> -??????? .of_match_table = stm32_iwdg_of_match,
>>>> +??????? .of_match_table = of_match_ptr(stm32_iwdg_of_match),
>>>> ????? },
>>>> ? };
>>>> ? module_platform_driver(stm32_iwdg_driver);
>>>>
>>>
>>
> 

^ permalink raw reply

* [PATCH 3/6] dt-bindings: xilinx_dma: add required xlnx,lengthregwidth property
From: Radhey Shyam Pandey @ 2018-06-20 13:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180620083653.17010-3-andrea.merello@gmail.com>


> -----Original Message-----
> From: dmaengine-owner at vger.kernel.org [mailto:dmaengine-
> owner at vger.kernel.org] On Behalf Of Andrea Merello
> Sent: Wednesday, June 20, 2018 2:07 PM
> To: vkoul at kernel.org; dan.j.williams at intel.com; Michal Simek
> <michals@xilinx.com>; Appana Durga Kedareswara Rao
> <appanad@xilinx.com>; dmaengine at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> Andrea Merello <andrea.merello@gmail.com>
> Subject: [PATCH 3/6] dt-bindings: xilinx_dma: add required
> xlnx,lengthregwidth property

dt-bindings: dmaengine: xilinx_dma

Please also include DT folks.
> 
> The width of the "length register" cannot be autodetected, and it is now
> specified with a DT property. Add DOC for it.
> 
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> ---
>  Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> index a2b8bfaec43c..acecdc5d8d47 100644
> --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> @@ -36,6 +36,8 @@ Required properties:
> 
>  Required properties for VDMA:
>  - xlnx,num-fstores: Should be the number of framebuffers as configured in
> h/w.
> +Required properties for AXI DMA:
> +- xlnx,lengthregwidth: Should be the width of the length register as
> configured in h/w.

One suggestion to be inline with IP property naming we can rename 
this prop to "xlnx,sg-length-width"? Please take a look at Xilinx tree
we have this feature added in the master branch. It would be good
to consolidate both implementations and upstream. Let me know 
if there are any followup queries. 
 
> 
>  Optional properties:
>  - xlnx,include-sg: Tells configured for Scatter-mode in
> --
> 2.17.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH V4] PCI: move early dump functionality from x86 arch into the common code
From: Andy Shevchenko @ 2018-06-20 13:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1528164985-14099-1-git-send-email-okaya@codeaurora.org>

On Tue, Jun 5, 2018 at 5:16 AM, Sinan Kaya <okaya@codeaurora.org> wrote:
> Move early dump functionality into common code so that it is available for
> all archtiectures. No need to carry arch specific reads around as the read
> hooks are already initialized by the time pci_setup_device() is getting
> called during scan.
>

It didn't break my setup on x86 at least. Thus,

Tested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  2 +-
>  arch/x86/include/asm/pci-direct.h               |  4 ---
>  arch/x86/kernel/setup.c                         |  5 ---
>  arch/x86/pci/common.c                           |  4 ---
>  arch/x86/pci/early.c                            | 44 -------------------------
>  drivers/pci/pci.c                               |  5 +++
>  drivers/pci/pci.h                               |  1 +
>  drivers/pci/probe.c                             | 19 +++++++++++
>  8 files changed, 26 insertions(+), 58 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e490902..e64f1d8 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2995,7 +2995,7 @@
>                         See also Documentation/blockdev/paride.txt.
>
>         pci=option[,option...]  [PCI] various PCI subsystem options:
> -               earlydump       [X86] dump PCI config space before the kernel
> +               earlydump       dump PCI config space before the kernel
>                                 changes anything
>                 off             [X86] don't probe for the PCI bus
>                 bios            [X86-32] force use of PCI BIOS, don't access
> diff --git a/arch/x86/include/asm/pci-direct.h b/arch/x86/include/asm/pci-direct.h
> index e1084f7..94597a3 100644
> --- a/arch/x86/include/asm/pci-direct.h
> +++ b/arch/x86/include/asm/pci-direct.h
> @@ -15,8 +15,4 @@ extern void write_pci_config_byte(u8 bus, u8 slot, u8 func, u8 offset, u8 val);
>  extern void write_pci_config_16(u8 bus, u8 slot, u8 func, u8 offset, u16 val);
>
>  extern int early_pci_allowed(void);
> -
> -extern unsigned int pci_early_dump_regs;
> -extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
> -extern void early_dump_pci_devices(void);
>  #endif /* _ASM_X86_PCI_DIRECT_H */
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 2f86d88..480f250 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -991,11 +991,6 @@ void __init setup_arch(char **cmdline_p)
>                 setup_clear_cpu_cap(X86_FEATURE_APIC);
>         }
>
> -#ifdef CONFIG_PCI
> -       if (pci_early_dump_regs)
> -               early_dump_pci_devices();
> -#endif
> -
>         e820__reserve_setup_data();
>         e820__finish_early_params();
>
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 563049c..d4ec117 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -22,7 +22,6 @@
>  unsigned int pci_probe = PCI_PROBE_BIOS | PCI_PROBE_CONF1 | PCI_PROBE_CONF2 |
>                                 PCI_PROBE_MMCONF;
>
> -unsigned int pci_early_dump_regs;
>  static int pci_bf_sort;
>  int pci_routeirq;
>  int noioapicquirk;
> @@ -599,9 +598,6 @@ char *__init pcibios_setup(char *str)
>                 pci_probe |= PCI_BIG_ROOT_WINDOW;
>                 return NULL;
>  #endif
> -       } else if (!strcmp(str, "earlydump")) {
> -               pci_early_dump_regs = 1;
> -               return NULL;
>         } else if (!strcmp(str, "routeirq")) {
>                 pci_routeirq = 1;
>                 return NULL;
> diff --git a/arch/x86/pci/early.c b/arch/x86/pci/early.c
> index e5f753c..f5fc953 100644
> --- a/arch/x86/pci/early.c
> +++ b/arch/x86/pci/early.c
> @@ -57,47 +57,3 @@ int early_pci_allowed(void)
>                         PCI_PROBE_CONF1;
>  }
>
> -void early_dump_pci_device(u8 bus, u8 slot, u8 func)
> -{
> -       u32 value[256 / 4];
> -       int i;
> -
> -       pr_info("pci 0000:%02x:%02x.%d config space:\n", bus, slot, func);
> -
> -       for (i = 0; i < 256; i += 4)
> -               value[i / 4] = read_pci_config(bus, slot, func, i);
> -
> -       print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 16, 1, value, 256, false);
> -}
> -
> -void early_dump_pci_devices(void)
> -{
> -       unsigned bus, slot, func;
> -
> -       if (!early_pci_allowed())
> -               return;
> -
> -       for (bus = 0; bus < 256; bus++) {
> -               for (slot = 0; slot < 32; slot++) {
> -                       for (func = 0; func < 8; func++) {
> -                               u32 class;
> -                               u8 type;
> -
> -                               class = read_pci_config(bus, slot, func,
> -                                                       PCI_CLASS_REVISION);
> -                               if (class == 0xffffffff)
> -                                       continue;
> -
> -                               early_dump_pci_device(bus, slot, func);
> -
> -                               if (func == 0) {
> -                                       type = read_pci_config_byte(bus, slot,
> -                                                                   func,
> -                                                              PCI_HEADER_TYPE);
> -                                       if (!(type & 0x80))
> -                                               break;
> -                               }
> -                       }
> -               }
> -       }
> -}
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 97acba7..04052dc 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -115,6 +115,9 @@ static bool pcie_ari_disabled;
>  /* If set, the PCIe ATS capability will not be used. */
>  static bool pcie_ats_disabled;
>
> +/* If set, the PCI config space of each device is printed during boot. */
> +bool pci_early_dump;
> +
>  bool pci_ats_disabled(void)
>  {
>         return pcie_ats_disabled;
> @@ -5805,6 +5808,8 @@ static int __init pci_setup(char *str)
>                                 pcie_ats_disabled = true;
>                         } else if (!strcmp(str, "noaer")) {
>                                 pci_no_aer();
> +                       } else if (!strcmp(str, "earlydump")) {
> +                               pci_early_dump = true;
>                         } else if (!strncmp(str, "realloc=", 8)) {
>                                 pci_realloc_get_opt(str + 8);
>                         } else if (!strncmp(str, "realloc", 7)) {
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index c358e7a0..c33265e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -7,6 +7,7 @@
>  #define PCI_VSEC_ID_INTEL_TBT  0x1234  /* Thunderbolt */
>
>  extern const unsigned char pcie_link_speed[];
> +extern bool pci_early_dump;
>
>  bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 56771f3..3678f0a 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1545,6 +1545,23 @@ static int pci_intx_mask_broken(struct pci_dev *dev)
>         return 0;
>  }
>
> +static void early_dump_pci_device(struct pci_dev *pdev)
> +{
> +       u32 value[256 / 4];
> +       int i;
> +
> +       if (!pci_early_dump)
> +               return;
> +
> +       pci_info(pdev, "config space:\n");
> +
> +       for (i = 0; i < 256; i += 4)
> +               pci_read_config_dword(pdev, i, &value[i / 4]);
> +
> +       print_hex_dump(KERN_INFO, "", DUMP_PREFIX_OFFSET, 16, 1, value,
> +                      256, false);
> +}
> +
>  /**
>   * pci_setup_device - Fill in class and map information of a device
>   * @dev: the device structure to fill
> @@ -1594,6 +1611,8 @@ int pci_setup_device(struct pci_dev *dev)
>         pci_printk(KERN_DEBUG, dev, "[%04x:%04x] type %02x class %#08x\n",
>                    dev->vendor, dev->device, dev->hdr_type, dev->class);
>
> +       early_dump_pci_device(dev);
> +
>         /* Need to have dev->class ready */
>         dev->cfg_size = pci_cfg_space_size(dev);
>
> --
> 2.7.4
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* [PATCH 4/5] arm64: dts: allwinner: h6: Use macros for R_CCU clock and reset indices
From: Chen-Yu Tsai @ 2018-06-20 13:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f17d1d31905cd3276d257053b8ed632de941494c.camel@aosc.io>

On Wed, Jun 20, 2018 at 9:11 PM, Icenowy Zheng <icenowy@aosc.io> wrote:
> ? 2018-06-20?? 13:15 +0800?Chen-Yu Tsai???
>> Now that the device tree binding headers for the R_CCU have been
>> merged,
>> we can use the macros, instead of raw numbers.
>>
>> Switch to R_CCU macros for clock and reset indices.
>>
>> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
>> ---
>>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> index c72da8cd9ef5..d85070f8c4a2 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
>> @@ -5,7 +5,9 @@
>>
>>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>>  #include <dt-bindings/clock/sun50i-h6-ccu.h>
>> +#include <dt-bindings/clock/sun50i-h6-r-ccu.h>
>>  #include <dt-bindings/reset/sun50i-h6-ccu.h>
>> +#include <dt-bindings/reset/sun50i-h6-r-ccu.h>
>>
>>  / {
>>       interrupt-parent = <&gic>;
>> @@ -198,7 +200,7 @@
>>                       reg = <0x07022000 0x400>;
>>                       interrupts = <GIC_SPI 105
>> IRQ_TYPE_LEVEL_HIGH>,
>>                                    <GIC_SPI 111
>> IRQ_TYPE_LEVEL_HIGH>;
>> -                     clocks = <&r_ccu 2>, <&osc24M>, <&osc32k>;
>> +                     clocks = <&r_ccu CLK_R_APB1>, <&osc24M>,
>> <&osc32k>;
>>                       clock-names = "apb", "hosc", "losc";
>>                       gpio-controller;
>>                       #gpio-cells = <3>;
>> @@ -208,6 +210,7 @@
>>                       r_i2c_pins: r-i2c {
>>                               pins = "PL0", "PL1";
>>                               function = "s_i2c";
>> +                             bias-pull-up;
>
> Should this be included in this patch?

Oops. I'll remove it either in the next version, or if everything
else checks out, when applying.

Thanks!
ChenYu

>
>>                       };
>>               };
>>
>> @@ -215,8 +218,8 @@
>>                       compatible = "allwinner,sun6i-a31-i2c";
>>                       reg = <0x07081400 0x400>;
>>                       interrupts = <GIC_SPI 107
>> IRQ_TYPE_LEVEL_HIGH>;
>> -                     clocks = <&r_ccu 8>;
>> -                     resets = <&r_ccu 4>;
>> +                     clocks = <&r_ccu CLK_R_APB2_I2C>;
>> +                     resets = <&r_ccu RST_R_APB2_I2C>;
>>                       pinctrl-names = "default";
>>                       pinctrl-0 = <&r_i2c_pins>;
>>                       status = "disabled";

^ permalink raw reply

* [PATCH V3 0/3] add iwdg2 support for stm32mp157c
From: Ludovic Barre @ 2018-06-20 13:51 UTC (permalink / raw)
  To: linux-arm-kernel

From: Ludovic Barre <ludovic.barre@st.com>

This patch series updates stm32_iwdg driver to manage config
by compatible. stm32mp1 config requires a pclk clock.

v3:
-remove stm32_iwdg_config structure, just assign the
 boolean directly to .dat

Ludovic Barre (3):
  watchdog: stm32: add pclk feature for stm32mp1
  ARM: dts: stm32: add iwdg2 support for stm32mp157c
  ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1

 .../devicetree/bindings/watchdog/st,stm32-iwdg.txt |  21 +++-
 arch/arm/boot/dts/stm32mp157c-ed1.dts              |   5 +
 arch/arm/boot/dts/stm32mp157c.dtsi                 |   8 ++
 drivers/watchdog/stm32_iwdg.c                      | 116 +++++++++++++--------
 4 files changed, 104 insertions(+), 46 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH V3 1/3] watchdog: stm32: add pclk feature for stm32mp1
From: Ludovic Barre @ 2018-06-20 13:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529502698-13263-1-git-send-email-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds config data to manage specific properties by
compatible. Adds stm32mp1 config which requires pclk clock.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 .../devicetree/bindings/watchdog/st,stm32-iwdg.txt |  21 +++-
 drivers/watchdog/stm32_iwdg.c                      | 116 +++++++++++++--------
 2 files changed, 91 insertions(+), 46 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
index cc13b10a..f07f6d89 100644
--- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
+++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt
@@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG)
 ---------------------------------
 
 Required properties:
-- compatible: "st,stm32-iwdg"
-- reg: physical base address and length of the registers set for the device
-- clocks: must contain a single entry describing the clock input
+- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg"
+- reg: Physical base address and length of the registers set for the device
+- clocks: Reference to the clock entry lsi. Additional pclk clock entry
+  is required only for st,stm32mp1-iwdg.
+- clock-names: Name of the clocks used.
+  "lsi" for st,stm32-iwdg
+  "pclk", "lsi" for st,stm32mp1-iwdg
 
 Optional Properties:
 - timeout-sec: Watchdog timeout value in seconds.
 
-Example:
+Examples:
 
 iwdg: watchdog at 40003000 {
 	compatible = "st,stm32-iwdg";
 	reg = <0x40003000 0x400>;
 	clocks = <&clk_lsi>;
+	clock-names = "lsi";
+	timeout-sec = <32>;
+};
+
+iwdg: iwdg at 5a002000 {
+	compatible = "st,stm32mp1-iwdg";
+	reg = <0x5a002000 0x400>;
+	clocks = <&rcc IWDG2>, <&clk_lsi>;
+	clock-names = "pclk", "lsi";
 	timeout-sec = <32>;
 };
diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
index c97ad56..e00e3b3 100644
--- a/drivers/watchdog/stm32_iwdg.c
+++ b/drivers/watchdog/stm32_iwdg.c
@@ -11,12 +11,13 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/watchdog.h>
 
@@ -54,11 +55,15 @@
 #define TIMEOUT_US	100000
 #define SLEEP_US	1000
 
+#define HAS_PCLK	true
+
 struct stm32_iwdg {
 	struct watchdog_device	wdd;
 	void __iomem		*regs;
-	struct clk		*clk;
+	struct clk		*clk_lsi;
+	struct clk		*clk_pclk;
 	unsigned int		rate;
+	bool			has_pclk;
 };
 
 static inline u32 reg_read(void __iomem *base, u32 reg)
@@ -133,6 +138,44 @@ static int stm32_iwdg_set_timeout(struct watchdog_device *wdd,
 	return 0;
 }
 
+static int stm32_iwdg_clk_init(struct platform_device *pdev,
+			       struct stm32_iwdg *wdt)
+{
+	u32 ret;
+
+	wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi");
+	if (IS_ERR(wdt->clk_lsi)) {
+		dev_err(&pdev->dev, "Unable to get lsi clock\n");
+		return PTR_ERR(wdt->clk_lsi);
+	}
+
+	/* optional peripheral clock */
+	if (wdt->has_pclk) {
+		wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk");
+		if (IS_ERR(wdt->clk_pclk)) {
+			dev_err(&pdev->dev, "Unable to get pclk clock\n");
+			return PTR_ERR(wdt->clk_pclk);
+		}
+
+		ret = clk_prepare_enable(wdt->clk_pclk);
+		if (ret) {
+			dev_err(&pdev->dev, "Unable to prepare pclk clock\n");
+			return ret;
+		}
+	}
+
+	ret = clk_prepare_enable(wdt->clk_lsi);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to prepare lsi clock\n");
+		clk_disable_unprepare(wdt->clk_pclk);
+		return ret;
+	}
+
+	wdt->rate = clk_get_rate(wdt->clk_lsi);
+
+	return 0;
+}
+
 static const struct watchdog_info stm32_iwdg_info = {
 	.options	= WDIOF_SETTIMEOUT |
 			  WDIOF_MAGICCLOSE |
@@ -147,49 +190,42 @@ static const struct watchdog_ops stm32_iwdg_ops = {
 	.set_timeout	= stm32_iwdg_set_timeout,
 };
 
+static const struct of_device_id stm32_iwdg_of_match[] = {
+	{ .compatible = "st,stm32-iwdg", .data = (void *)!HAS_PCLK },
+	{ .compatible = "st,stm32mp1-iwdg", .data = (void *)HAS_PCLK },
+	{ /* end node */ }
+};
+MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
+
 static int stm32_iwdg_probe(struct platform_device *pdev)
 {
 	struct watchdog_device *wdd;
+	const struct of_device_id *match;
 	struct stm32_iwdg *wdt;
 	struct resource *res;
-	void __iomem *regs;
-	struct clk *clk;
 	int ret;
 
+	match = of_match_device(stm32_iwdg_of_match, &pdev->dev);
+	if (!match)
+		return -ENODEV;
+
+	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
+	if (!wdt)
+		return -ENOMEM;
+
+	wdt->has_pclk = match->data;
+
 	/* This is the timer base. */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(regs)) {
+	wdt->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(wdt->regs)) {
 		dev_err(&pdev->dev, "Could not get resource\n");
-		return PTR_ERR(regs);
+		return PTR_ERR(wdt->regs);
 	}
 
-	clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(clk)) {
-		dev_err(&pdev->dev, "Unable to get clock\n");
-		return PTR_ERR(clk);
-	}
-
-	ret = clk_prepare_enable(clk);
-	if (ret) {
-		dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk);
+	ret = stm32_iwdg_clk_init(pdev, wdt);
+	if (ret)
 		return ret;
-	}
-
-	/*
-	 * Allocate our watchdog driver data, which has the
-	 * struct watchdog_device nested within it.
-	 */
-	wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL);
-	if (!wdt) {
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	/* Initialize struct stm32_iwdg. */
-	wdt->regs = regs;
-	wdt->clk = clk;
-	wdt->rate = clk_get_rate(clk);
 
 	/* Initialize struct watchdog_device. */
 	wdd = &wdt->wdd;
@@ -217,7 +253,8 @@ static int stm32_iwdg_probe(struct platform_device *pdev)
 
 	return 0;
 err:
-	clk_disable_unprepare(clk);
+	clk_disable_unprepare(wdt->clk_lsi);
+	clk_disable_unprepare(wdt->clk_pclk);
 
 	return ret;
 }
@@ -227,23 +264,18 @@ static int stm32_iwdg_remove(struct platform_device *pdev)
 	struct stm32_iwdg *wdt = platform_get_drvdata(pdev);
 
 	watchdog_unregister_device(&wdt->wdd);
-	clk_disable_unprepare(wdt->clk);
+	clk_disable_unprepare(wdt->clk_lsi);
+	clk_disable_unprepare(wdt->clk_pclk);
 
 	return 0;
 }
 
-static const struct of_device_id stm32_iwdg_of_match[] = {
-	{ .compatible = "st,stm32-iwdg" },
-	{ /* end node */ }
-};
-MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match);
-
 static struct platform_driver stm32_iwdg_driver = {
 	.probe		= stm32_iwdg_probe,
 	.remove		= stm32_iwdg_remove,
 	.driver = {
 		.name	= "iwdg",
-		.of_match_table = stm32_iwdg_of_match,
+		.of_match_table = of_match_ptr(stm32_iwdg_of_match),
 	},
 };
 module_platform_driver(stm32_iwdg_driver);
-- 
2.7.4

^ permalink raw reply related

* [PATCH V3 2/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c
From: Ludovic Barre @ 2018-06-20 13:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529502698-13263-1-git-send-email-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

This patch adds independent watchdog support for stm32mp157c.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 7d17538..95cc166 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -784,6 +784,14 @@
 			status = "disabled";
 		};
 
+		iwdg2: watchdog at 5a002000 {
+			compatible = "st,stm32mp1-iwdg";
+			reg = <0x5a002000 0x400>;
+			clocks = <&rcc IWDG2>, <&rcc CK_LSI>;
+			clock-names = "pclk", "lsi";
+			status = "disabled";
+		};
+
 		usbphyc: usbphyc at 5a006000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-- 
2.7.4

^ permalink raw reply related

* [PATCH V3 3/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1
From: Ludovic Barre @ 2018-06-20 13:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1529502698-13263-1-git-send-email-ludovic.Barre@st.com>

From: Ludovic Barre <ludovic.barre@st.com>

This patch activates independent watchdog support for
stm32mp157c board.

Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
 arch/arm/boot/dts/stm32mp157c-ed1.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
index ae33653..8af263a 100644
--- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
@@ -68,6 +68,11 @@
 	status = "okay";
 };
 
+&iwdg2 {
+	timeout-sec = <32>;
+	status = "okay";
+};
+
 &uart4 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart4_pins_a>;
-- 
2.7.4

^ permalink raw reply related

* Dynamic ftrace self test broken on ARM
From: Stefan Agner @ 2018-06-20 13:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180619091735.7aec75d0@gandalf.local.home>

On 19.06.2018 15:17, Steven Rostedt wrote:
> On Tue, 19 Jun 2018 10:16:19 +0200
> Stefan Agner <stefan@agner.ch> wrote:
> 
>> > I'm guessing that it boots fine with CONFIG_FTRACE_STARTUP_TEST=n? Can
>> > you try disable the tracers to see if it's the function graph or
>> > function tracer that is causing the issue? That is, turn off
>> > CONFIG_FUNCTION_GRAPH_TRACER and test it again, and if that crashes,
>> > turn off CONFIG_FUNCTION_TRACER to make sure the crash goes away there
>> > too.
>>
>> Without CONFIG_FTRACE_STARTUP_TEST the kernel boots fine.
>>
>> CONFIG_FUNCTION_TRACER=y
>> # CONFIG_FUNCTION_GRAPH_TRACER is not set
>> # CONFIG_SCHED_TRACER is not set
>> CONFIG_FTRACE_STARTUP_TEST=y
> 
> OK, so it's not a graph tracer issue, but a function tracer issue.
> 
>>
>> Crashes with the same stack trace.
>>
>> # CONFIG_FUNCTION_TRACER is not set
>> CONFIG_SCHED_TRACER=y
>> CONFIG_FTRACE_STARTUP_TEST=y
>>
>> Runs tracer tests and boots fine.
> 
> Thanks. Did this ever work? And if so, perhaps you have time to perform
> a bisect. If you have a ktest setup, you can have it run the bisect for
> you (over night).

v4.9 seems to work, so I started bisecting. It turned out that commit
6f05d0761af6 ("ARM: 8668/1: ftrace: Fix dynamic ftrace with DEBUG_RODATA
and !FRAME_POINTER") broke it, introduced during the v4.12 merge window.

I did not look closer into it yet.

Abel, maybe you see what is going on here?

--
Stefan

^ permalink raw reply

* [PATCH 1/4] ARM: Introduce ability to enable ACR::IBE on Cortex-A8 for CVE-2017-5715
From: Fabio Estevam @ 2018-06-20 14:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180612202411.29798-2-nm@ti.com>

On Tue, Jun 12, 2018 at 5:24 PM, Nishanth Menon <nm@ti.com> wrote:
> As recommended by Arm in [1], IBE[2] has to be enabled unconditionally
> for BPIALL to be functional on Cortex-A8 processors. Provide a config
> option for platforms to enable this option based on impact analysis
> for products.
>
> NOTE: This patch in itself is NOT the final solution, this requires:
> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>    provide direct access to ACR register.
> b) Operating Systems such as Linux to provide adequate workaround in the right
>    locations.
> c) This workaround applies to only the boot processor. It is important
>    to apply workaround as necessary (context-save-restore) around low
>    power context loss OR additional processors as necessary in either
>    firmware support OR elsewhere in OS.
>
> [1] https://developer.arm.com/support/security-update
> [2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0344k/Bgbffjhh.html
>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Andre Przywara <Andre.Przywara@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

On a imx51-babbage board:

Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply

* Dynamic ftrace self test broken on ARM
From: Steven Rostedt @ 2018-06-20 14:13 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5e26cebbb2ebeb87fdc808509881736b@agner.ch>

On Wed, 20 Jun 2018 15:51:55 +0200
Stefan Agner <stefan@agner.ch> wrote:


> v4.9 seems to work, so I started bisecting. It turned out that commit
> 6f05d0761af6 ("ARM: 8668/1: ftrace: Fix dynamic ftrace with DEBUG_RODATA
> and !FRAME_POINTER") broke it, introduced during the v4.12 merge window.

That patch doesn't appear to be the cause. It could have been a failed
bisect. Does the commit before that commit work? Does that commit fail?

It may be due to some other RODATA change though. That is actually one
of my thoughts when looking at the bug.

-- Steve

^ permalink raw reply

* [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715
From: Fabio Estevam @ 2018-06-20 14:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180612202411.29798-3-nm@ti.com>

On Tue, Jun 12, 2018 at 5:24 PM, Nishanth Menon <nm@ti.com> wrote:
> As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
> needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
> be done unconditionally for Cortex-A15 processors. Provide a config
> option for platforms to enable this option based on impact analysis
> for products.
>
> NOTE: This patch in itself is NOT the final solution, this requires:
> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>    provide direct access to ACR register.
> b) Operating Systems such as Linux to provide adequate workaround in the
>    right locations.
> c) This workaround applies to only the boot processor. It is important
>    to apply workaround as necessary (context-save-restore) around low
>    power context loss OR additional processors as necessary in either
>    firmware support OR elsewhere in OS.
>
> [1] https://developer.arm.com/support/security-update
> [2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0438c/BABGHIBG.html
>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Andre Przywara <Andre.Przywara@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

On a imx51-babbage board:

Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

^ permalink raw reply

* KVM guest sometimes failed to boot because of kernel stack overflow if KPTI is enabled on a hisilicon ARM64 platform.
From: Wei Xu @ 2018-06-20 14:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi All,

We have observed KVM guest sometimes failed to boot because of kernel stack
overflow if KPTI is enabled on a hisilicon arm64 platform.

We also tested with different kernel version and found it is only
happened if the KPTI and KVM(enable-kvm & cpu=host) are enabled on the 
guest.
The detail result is as below table.

+---------+----------+--------+------------+-------------------+
      |  host   |host KPTI | guest  | guest KPTI | kvm guest         |
      |  kernel |enabled   | kernel | enabled    | booting result    |
+---------+----------+--------+------------+-------------------+
      |  4.17   |     Y    |  4.17  |     Y      |  stack overflow   |
+---------+----------+--------+------------+-------------------+
      |  4.17   |     Y    |  4.16  |     NA     | OK          |
+---------+----------+--------+------------+-------------------+
      |  4.16   |     NA   |  4.17  |     Y      |  stack overflow   |
+---------+----------+--------+------------+-------------------+
      |  4.16   |     NA   |  4.16  |     NA     | OK          |
+---------+----------+--------+------------+-------------------+

A simple walk-around is adding this platform into the "kpti_safe_list".
But it does not resolve the issue indeed.
Could you please share any hint how to resolve this kind issue?
Thanks!

Another issue we found is "kpti_install_ng_mappings" will be invoked
even "kpti=off" has been added in the kernel command line. Is that expected?
This is because "kpti" is not a *early* param that "init_cpu_features" will
be invoked before parsing the param.

The command we are using to run the guest is as:

     ./qemu-system-aarch64 -machine virt,kernel_irqchip=on,gic-version=3 
-cpu host
     -enable-kvm -smp 1 -m 1024 -kernel ./Image -initrd 
../mini-rootfs-arm64.cpio.gz
     -nographic -append "rdinit=init console=ttyAMA0 
earlycon=pl011,0x9000000"

The log is as below:

         [    0.000000] Booting Linux on physical CPU 0x0000000000 
[0x480fd010]
         [    0.000000] Linux version 4.17.0-45864-g29dcea8-dirty 
(joyx at Turing-Arch-b) (gcc version 4.9.1 20140505 (prerelease) 
(crosstool-NG linaro-1.13.1-4.9-2014.05 - Linaro GCC 4.9-2014.05)) #6 
SMP PREEMPT Fri Jun 15 21:39:52 CST 2018
         [    0.000000] Machine model: linux,dummy-virt
         [    0.000000] earlycon: pl11 at MMIO 0x0000000009000000 
(options '')
         [    0.000000] bootconsole [pl11] enabled
         [    0.000000] efi: Getting EFI parameters from FDT:
         [    0.000000] efi: UEFI not found.
         [    0.000000] cma: Reserved 16 MiB at 0x000000007f000000
         [    0.000000] NUMA: No NUMA configuration found
         [    0.000000] NUMA: Faking a node at [mem 
0x0000000000000000-0x000000007fffffff]
         [    0.000000] NUMA: NODE_DATA [mem 0x7efeb300-0x7efecdff]
         [    0.000000] Zone ranges:
         [    0.000000]   DMA32    [mem 
0x0000000040000000-0x000000007fffffff]
         [    0.000000]   Normal   empty
         [    0.000000] Movable zone start for each node
         [    0.000000] Early memory node ranges
         [    0.000000]   node   0: [mem 
0x0000000040000000-0x000000007fffffff]
         [    0.000000] Initmem setup node 0 [mem 
0x0000000040000000-0x000000007fffffff]
         [    0.000000] psci: probing for conduit method from DT.
         [    0.000000] psci: PSCIv1.0 detected in firmware.
         [    0.000000] psci: Using standard PSCI v0.2 function IDs
         [    0.000000] psci: Trusted OS migration not required
         [    0.000000] psci: SMC Calling Convention v1.1
         [    0.000000] random: get_random_bytes called from 
start_kernel+0xa8/0x418 with crng_init=0
         [    0.000000] percpu: Embedded 24 pages/cpu @        (ptrval) 
s57984 r8192 d32128 u98304
         [    0.000000] Detected VIPT I-cache on CPU0
         [    0.000000] CPU features: detected: Kernel page table 
isolation (KPTI)
         [    0.000000] CPU features: detected: Hardware dirty bit 
management
         [    0.000000] Built 1 zonelists, mobility grouping on.  Total 
pages: 258048
         [    0.000000] Policy zone: DMA32
         [    0.000000] Kernel command line: rdinit=init console=ttyAMA0 
earlycon=pl011,0x9000000
         [    0.000000] Memory: 968436K/1048576K available (10044K 
kernel code, 1328K rwdata, 4840K rodata, 1216K init, 409K bss, 63756K 
reserved, 16384K cma-reserved)
         [    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, 
CPUs=1, Nodes=1
         [    0.000000] Preemptible hierarchical RCU implementation.
         [    0.000000]     RCU restricting CPUs from NR_CPUS=128 to 
nr_cpu_ids=1.
         [    0.000000]     Tasks RCU enabled.
         [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=16, 
nr_cpu_ids=1
         [    0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
         [    0.000000] GICv3: Distributor has no Range Selector support
         [    0.000000] GICv3: no VLPI support, no direct LPI support
         [    0.000000] ITS [mem 0x08080000-0x0809ffff]
         [    0.000000] ITS at 0x0000000008080000: allocated 8192 Devices 
@7d830000 (indirect, esz 8, psz 64K, shr 1)
         [    0.000000] ITS at 0x0000000008080000: allocated 8192 Interrupt 
Collections @7d840000 (flat, esz 8, psz 64K, shr 1)
         [    0.000000] GIC: using LPI property table @0x000000007d850000
         [    0.000000] ITS: Allocated 1792 chunks for LPIs
         [    0.000000] GICv3: CPU0: found redistributor 0 region 
0:0x00000000080a0000
         [    0.000000] CPU0: using LPI pending table @0x000000007d860000
         [    0.000000] GIC: PPI11 is secure or misconfigured
         [    0.000000] arch_timer: WARNING: Invalid trigger for IRQ3, 
assuming level low
         [    0.000000] arch_timer: WARNING: Please fix your firmware
         [    0.000000] arch_timer: cp15 timer(s) running at 100.00MHz 
(virt).
         [    0.000000] clocksource: arch_sys_counter: mask: 
0xffffffffffffff max_cycles: 0x171024e7e0, max_idle_ns: 440795205315 ns
         [    0.000001] sched_clock: 56 bits at 100MHz, resolution 10ns, 
wraps every 4398046511100ns
         [    0.000854] Console: colour dummy device 80x25
         [    0.001423] Calibrating delay loop (skipped), value 
calculated using timer frequency.. 200.00 BogoMIPS (lpj=400000)
         [    0.002478] pid_max: default: 32768 minimum: 301
         [    0.002962] Security Framework initialized
         [    0.003541] Dentry cache hash table entries: 131072 (order: 
8, 1048576 bytes)
         [    0.004347] Inode-cache hash table entries: 65536 (order: 7, 
524288 bytes)
         [    0.005058] Mount-cache hash table entries: 2048 (order: 2, 
16384 bytes)
         [    0.005844] Mountpoint-cache hash table entries: 2048 
(order: 2, 16384 bytes)
         [    0.025949] ASID allocator initialised with 32768 entries
         [    0.029958] Hierarchical SRCU implementation.
         [    0.034328] Platform MSI: its domain created
         [    0.034787] PCI/MSI: /intc/its domain created
         [    0.035359] EFI services will not be available.
         [    0.037987] smp: Bringing up secondary CPUs ...
         [    0.038454] smp: Brought up 1 node, 1 CPU
         [    0.038859] SMP: Total of 1 processors activated.
         [    0.039338] CPU features: detected: GIC system register CPU 
interface
         [    0.039988] CPU features: detected: Privileged Access Never
         [    0.040560] CPU features: detected: User Access Override
         [    0.041093] CPU features: detected: RAS Extension Support
         [    0.042947] Insufficient stack space to handle exception!
         [    0.042949] ESR: 0x96000046 -- DABT (current EL)
         [    0.043963] FAR: 0xffff0000093a80e0
         [    0.045794] Task stack: [0xffff0000093a8000..0xffff0000093ac000]
         [    0.052181] IRQ stack: [0xffff000008000000..0xffff000008004000]
         [    0.058572] Overflow stack: 
[0xffff80003efce2f0..0xffff80003efcf2f0]
         [    0.065068] CPU: 0 PID: 12 Comm: migration/0 Not tainted 
4.17.0-45864-g29dcea8-dirty #6
         [    0.073138] Hardware name: linux,dummy-virt (DT)
         [    0.077831] pstate: 604003c5 (nZCv DAIF +PAN -UAO)
         [    0.082661] pc : el1_sync+0x0/0xb0
         [    0.086152] lr : kpti_install_ng_mappings+0x120/0x214
         [    0.091219] sp : ffff0000093a80e0
         [    0.094589] x29: ffff0000093abce0 x28: ffff000008ea9000
         [    0.100004] x27: ffff000008ea9000 x26: ffff0000091f7000
         [    0.105424] x25: ffff00000906d000 x24: ffff000009191000
         [    0.110733] x23: ffff000008ea9000 x22: 0000000041190000
         [    0.116148] x21: ffff0000091f7000 x20: 0000000000000000
         [    0.121564] x19: ffff000009190000 x18: 000000003455d99d
         [    0.126977] x17: 0000000000000001 x16: 00f8000040ffff13
         [    0.132288] x15: 000000007eff6000 x14: 000000007eff6000
         [    0.137704] x13: 00f800007fe00f11 x12: 000000007eff8000
         [    0.143013] x11: 000000007eff8000 x10: 0000000000000000
         [    0.148426] x9 : 000000007eff9000 x8 : 000000007eff9000
         [    0.153841] x7 : 0000000000000000 x6 : 00000000411f8000
         [    0.159154] x5 : 00000000411f8000 x4 : 0000000040a443d4
         [    0.164567] x3 : 00000000411f7000 x2 : 00000000411f7000
         [    0.169981] x1 : ffff00000906d7b0 x0 : ffff80003da61c00
         [    0.175395] Kernel panic - not syncing: kernel stack overflow
         [    0.181178] CPU: 0 PID: 12 Comm: migration/0 Not tainted 
4.17.0-45864-g29dcea8-dirty #6
         [    0.189248] Hardware name: linux,dummy-virt (DT)
         [    0.193945] Call trace:
         [    0.196470]  dump_backtrace+0x0/0x180
         [    0.200201]  show_stack+0x14/0x1c
         [    0.203574]  dump_stack+0x90/0xb0
         [    0.206946]  panic+0x138/0x2a0
         [    0.210075]  __stack_chk_fail+0x0/0x18
         [    0.213922]  handle_bad_stack+0x118/0x124
         [    0.218012]  __bad_stack+0x88/0x8c
         [    0.221393]  el1_sync+0x0/0xb0
         [    0.224520] Unable to handle kernel paging request at 
virtual address ffff0000093abce0
         [    0.232586] Mem abort info:
         [    0.235362]   ESR = 0x96000006
         [    0.238488]   Exception class = DABT (current EL), IL = 32 bits
         [    0.244506]   SET = 0, FnV = 0
         [    0.247632]   EA = 0, S1PTW = 0
         [    0.250873] Data abort info:
         [    0.253765]   ISV = 0, ISS = 0x00000006
         [    0.257725]   CM = 0, WnR = 0
         [    0.260735] swapper pgtable: 4k pages, 48-bit VAs, pgdp 
=         (ptrval)


Best Regards,
Wei

^ permalink raw reply

* [RFC PATCH 1/2] spi: Add QuadSPI driver for Atmel SAMA5D2
From: Piotr Bugalski @ 2018-06-20 14:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180619151506.GG11230@sirena.org.uk>

Hi Mark,

Thank you very much for quick answer.

On Tue, 19 Jun 2018, Mark Brown wrote:

> On Mon, Jun 18, 2018 at 06:21:23PM +0200, Piotr Bugalski wrote:
>
>> +static int atmel_qspi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>> +{
>> +	return 0;
>> +}
>
> If this can be empty should we adjust the callers to allow it to just be
> omitted?
>

If I remember well some commits ago spi-mem required even empty 
adjust_op_size. Now it seems unnecessary, but I forgot to remove the 
code. I will fix it in next version.

>> +static int atmel_qspi_remove(struct platform_device *pdev)
>> +{
>> +	struct spi_controller *ctrl = platform_get_drvdata(pdev);
>> +	struct atmel_qspi *aq = spi_controller_get_devdata(ctrl);
>> +
>> +	qspi_writel(aq, QSPI_CR, QSPI_CR_QSPIDIS);
>> +	clk_disable_unprepare(aq->clk);
>> +
>> +	spi_unregister_controller(ctrl);
>> +
>> +	return 0;
>> +}
>
> You should unregister the controller before disabling the hardware,
> otherwise something could come in and try to start an operation on the
> controller (or already be running one) while the hardware is disabled
> which might blow up.
>

Sure, deinit should be done in reverse order of init, you are perfectly 
right, just my mistake. I'll fix it in next version.

Best Regards,
Piotr

^ permalink raw reply

* [PATCH 3/6] dt-bindings: xilinx_dma: add required xlnx,lengthregwidth property
From: Radhey Shyam Pandey @ 2018-06-20 14:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <DM6PR02MB43610E55E255E9DA5B9E4630C7770@DM6PR02MB4361.namprd02.prod.outlook.com>

> -----Original Message-----
> From: Radhey Shyam Pandey
> Sent: Wednesday, June 20, 2018 7:13 PM
> To: Andrea Merello <andrea.merello@gmail.com>; vkoul at kernel.org;
> dan.j.williams at intel.com; Michal Simek <michals@xilinx.com>; Appana Durga
> Kedareswara Rao <appanad@xilinx.com>; dmaengine at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org
> Subject: RE: [PATCH 3/6] dt-bindings: xilinx_dma: add required
> xlnx,lengthregwidth property
> 
> 
> > -----Original Message-----
> > From: dmaengine-owner at vger.kernel.org [mailto:dmaengine-
> > owner at vger.kernel.org] On Behalf Of Andrea Merello
> > Sent: Wednesday, June 20, 2018 2:07 PM
> > To: vkoul at kernel.org; dan.j.williams at intel.com; Michal Simek
> > <michals@xilinx.com>; Appana Durga Kedareswara Rao
> > <appanad@xilinx.com>; dmaengine at vger.kernel.org
> > Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> > Andrea Merello <andrea.merello@gmail.com>
> > Subject: [PATCH 3/6] dt-bindings: xilinx_dma: add required
> > xlnx,lengthregwidth property
> 
> dt-bindings: dmaengine: xilinx_dma
> 
> Please also include DT folks.
> >
> > The width of the "length register" cannot be autodetected, and it is now
> > specified with a DT property. Add DOC for it.
> >
> > Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > index a2b8bfaec43c..acecdc5d8d47 100644
> > --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> > @@ -36,6 +36,8 @@ Required properties:
> >
> >  Required properties for VDMA:
> >  - xlnx,num-fstores: Should be the number of framebuffers as configured in
> > h/w.
> > +Required properties for AXI DMA:
> > +- xlnx,lengthregwidth: Should be the width of the length register as
> > configured in h/w.
> 
> One suggestion to be inline with IP property naming we can rename
> this prop to "xlnx,sg-length-width"? Please take a look at Xilinx tree
> we have this feature added in the master branch. It would be good
> to consolidate both implementations and upstream. Let me know
> if there are any followup queries.

It should be ok to cherrypick 3/6 and 4/6 (xlnx,sg-length-width)
from Xilinx tree and include it in your v2 patch series. 

> 
> >
> >  Optional properties:
> >  - xlnx,include-sg: Tells configured for Scatter-mode in
> > --
> > 2.17.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* KVM guest sometimes failed to boot because of kernel stack overflow if KPTI is enabled on a hisilicon ARM64 platform.
From: Will Deacon @ 2018-06-20 14:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5B2A6218.3030201@hisilicon.com>

Hi Wei,

On Wed, Jun 20, 2018 at 10:18:00PM +0800, Wei Xu wrote:
> We have observed KVM guest sometimes failed to boot because of kernel stack
> overflow if KPTI is enabled on a hisilicon arm64 platform.
> 
> We also tested with different kernel version and found it is only
> happened if the KPTI and KVM(enable-kvm & cpu=host) are enabled on the
> guest.
> The detail result is as below table.
> 
> +---------+----------+--------+------------+-------------------+
>      |  host   |host KPTI | guest  | guest KPTI | kvm guest         |
>      |  kernel |enabled   | kernel | enabled    | booting result    |
> +---------+----------+--------+------------+-------------------+
>      |  4.17   |     Y    |  4.17  |     Y      |  stack overflow   |
> +---------+----------+--------+------------+-------------------+
>      |  4.17   |     Y    |  4.16  |     NA     | OK          |
> +---------+----------+--------+------------+-------------------+
>      |  4.16   |     NA   |  4.17  |     Y      |  stack overflow   |
> +---------+----------+--------+------------+-------------------+
>      |  4.16   |     NA   |  4.16  |     NA     | OK          |
> +---------+----------+--------+------------+-------------------+
> 
> A simple walk-around is adding this platform into the "kpti_safe_list".
> But it does not resolve the issue indeed.
> Could you please share any hint how to resolve this kind issue?
> Thanks!
> 
> Another issue we found is "kpti_install_ng_mappings" will be invoked
> even "kpti=off" has been added in the kernel command line. Is that expected?
> This is because "kpti" is not a *early* param that "init_cpu_features" will
> be invoked before parsing the param.

That sounds like a straightforward bug, which means we should use
early_param instead of __setup. I assume that doesn't fix your crash,
though?

> The command we are using to run the guest is as:
> 
>     ./qemu-system-aarch64 -machine virt,kernel_irqchip=on,gic-version=3 -cpu
> host
>     -enable-kvm -smp 1 -m 1024 -kernel ./Image -initrd
> ../mini-rootfs-arm64.cpio.gz
>     -nographic -append "rdinit=init console=ttyAMA0
> earlycon=pl011,0x9000000"
> 
> The log is as below:
> 
>         [    0.000000] Booting Linux on physical CPU 0x0000000000
> [0x480fd010]
>         [    0.000000] Linux version 4.17.0-45864-g29dcea8-dirty
> (joyx at Turing-Arch-b) (gcc version 4.9.1 20140505 (prerelease) (crosstool-NG
> linaro-1.13.1-4.9-2014.05 - Linaro GCC 4.9-2014.05)) #6 SMP PREEMPT Fri Jun
> 15 21:39:52 CST 2018

^^^ This is reproducible with vanilla v4.17 and defconfig, right?

>         [    0.038859] SMP: Total of 1 processors activated.
>         [    0.039338] CPU features: detected: GIC system register CPU
> interface
>         [    0.039988] CPU features: detected: Privileged Access Never
>         [    0.040560] CPU features: detected: User Access Override
>         [    0.041093] CPU features: detected: RAS Extension Support
>         [    0.042947] Insufficient stack space to handle exception!
>         [    0.042949] ESR: 0x96000046 -- DABT (current EL)
>         [    0.043963] FAR: 0xffff0000093a80e0
>         [    0.045794] Task stack: [0xffff0000093a8000..0xffff0000093ac000]
>         [    0.052181] IRQ stack: [0xffff000008000000..0xffff000008004000]
>         [    0.058572] Overflow stack:
> [0xffff80003efce2f0..0xffff80003efcf2f0]
>         [    0.065068] CPU: 0 PID: 12 Comm: migration/0 Not tainted
> 4.17.0-45864-g29dcea8-dirty #6
>         [    0.073138] Hardware name: linux,dummy-virt (DT)
>         [    0.077831] pstate: 604003c5 (nZCv DAIF +PAN -UAO)
>         [    0.082661] pc : el1_sync+0x0/0xb0
>         [    0.086152] lr : kpti_install_ng_mappings+0x120/0x214

Can you use scripts/faddr2line to find out which line of code the lr is
pointing at, please? It would be interesting to know if we managed to
install the idmap.

Hmm, I wonder if this is at all related to RAS, since we've just enabled
that and if we take a fault whilst rewriting swapper then we're going to
get stuck. What happens if you set CONFIG_ARM64_RAS_EXTN=n in the guest?

Will

^ permalink raw reply

* [PATCH 5/6] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather
From: Radhey Shyam Pandey @ 2018-06-20 14:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180620083653.17010-5-andrea.merello@gmail.com>

> -----Original Message-----
> From: dmaengine-owner at vger.kernel.org [mailto:dmaengine-
> owner at vger.kernel.org] On Behalf Of Andrea Merello
> Sent: Wednesday, June 20, 2018 2:07 PM
> To: vkoul at kernel.org; dan.j.williams at intel.com; Michal Simek
> <michals@xilinx.com>; Appana Durga Kedareswara Rao
> <appanad@xilinx.com>; dmaengine at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
> Andrea Merello <andrea.merello@gmail.com>
> Subject: [PATCH 5/6] dmaengine: xilinx_dma: autodetect whether the HW
> supports scatter-gather
> 
> The HW can be either direct-access or scatter-gather version. These are
> SW incompatible.
> 
> The driver can handle both version: a DT property was used to
> tell the driver whether to assume the HW is is scatter-gather mode.
> 
> This patch makes the driver to autodetect this information. The DT
> property is not required anymore.
> 
> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
> ---
>  drivers/dma/xilinx/xilinx_dma.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index bdbc8ba9092a..8c6e818e596f 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -86,6 +86,7 @@
>  #define XILINX_DMA_DMASR_DMA_DEC_ERR		BIT(6)
>  #define XILINX_DMA_DMASR_DMA_SLAVE_ERR		BIT(5)
>  #define XILINX_DMA_DMASR_DMA_INT_ERR		BIT(4)
> +#define XILINX_DMA_DMASR_SG_MASK		BIT(3)
>  #define XILINX_DMA_DMASR_IDLE			BIT(1)
>  #define XILINX_DMA_DMASR_HALTED		BIT(0)
>  #define XILINX_DMA_DMASR_DELAY_MASK		GENMASK(31, 24)
> @@ -407,7 +408,6 @@ struct xilinx_dma_config {
>   * @dev: Device Structure
>   * @common: DMA device structure
>   * @chan: Driver specific DMA channel
> - * @has_sg: Specifies whether Scatter-Gather is present or not
>   * @mcdma: Specifies whether Multi-Channel is present or not
>   * @flush_on_fsync: Flush on frame sync
>   * @ext_addr: Indicates 64 bit addressing is supported by dma device
> @@ -426,7 +426,6 @@ struct xilinx_dma_device {
>  	struct device *dev;
>  	struct dma_device common;
>  	struct xilinx_dma_chan
> *chan[XILINX_DMA_MAX_CHANS_PER_DEVICE];
> -	bool has_sg;
>  	bool mcdma;
>  	u32 flush_on_fsync;
>  	bool ext_addr;
> @@ -2391,7 +2390,6 @@ static int xilinx_dma_chan_probe(struct
> xilinx_dma_device *xdev,
> 
>  	chan->dev = xdev->dev;
>  	chan->xdev = xdev;
> -	chan->has_sg = xdev->has_sg;
>  	chan->desc_pendingcount = 0x0;
>  	chan->ext_addr = xdev->ext_addr;
>  	/* This variable ensures that descriptors are not
> @@ -2488,6 +2486,13 @@ static int xilinx_dma_chan_probe(struct
> xilinx_dma_device *xdev,
>  		chan->stop_transfer = xilinx_dma_stop_transfer;
>  	}
> 
> +	/* check if SG is enabled */
> +	if (dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
> +		XILINX_DMA_DMASR_SG_MASK)
I think SGIncld mask is only applicable for AXI DMA and CDMA IP.
For VDMA IP this bit is reserved. 
.
> +		chan->has_sg = true;
> +	dev_dbg(chan->dev, "ch %d: SG %s\n", chan->id,
> +		chan->has_sg ? "enabled" : "disabled");
> +
>  	/* Initialize the tasklet */
>  	tasklet_init(&chan->tasklet, xilinx_dma_do_tasklet,
>  			(unsigned long)chan);
> @@ -2626,7 +2631,6 @@ static int xilinx_dma_probe(struct platform_device
> *pdev)
>  		return PTR_ERR(xdev->regs);
> 
>  	/* Retrieve the DMA engine properties from the device tree */
> -
Unrelated change

>  	if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
>  		xdev->mcdma = of_property_read_bool(node, "xlnx,mcdma");
>  		err = of_property_read_u32(node, "xlnx,lengthregwidth",
> --
> 2.17.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [RFC PATCH 2/2] dt-bindings: spi: QuadSPI driver for Atmel SAMA5D2 documentation
From: Boris Brezillon @ 2018-06-20 14:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180618162124.21749-3-bugalski.piotr@gmail.com>

Hi Piotr,

On Mon, 18 Jun 2018 18:21:24 +0200
Piotr Bugalski <bugalski.piotr@gmail.com> wrote:

> Documentation for DT-binding change.
> 
> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com>

I'm pretty sure I didn't make a single suggestion about the DT
bindings you use here ;-).

> Signed-off-by: Piotr Bugalski <pbu@cryptera.com>
> 
> ---
>  .../devicetree/bindings/spi/spi_atmel-qspi.txt     | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi_atmel-qspi.txt

I'll comment on this aspect in more details when replying to the cover
letter, but I think you should re-use the bindings defined in
Documentation/devicetree/bindings/mtd/atmel-quadspi.txt (IOW, move the
existing file to the Documentation/devicetree/bindings/spi directory).

It's the same HW block, and just because you develop a new driver to
replace the old one doesn't mean you should have 2 different bindings in
parallel.

> 
> diff --git a/Documentation/devicetree/bindings/spi/spi_atmel-qspi.txt b/Documentation/devicetree/bindings/spi/spi_atmel-qspi.txt
> new file mode 100644
> index 000000000000..d52b534c9c2b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi_atmel-qspi.txt
> @@ -0,0 +1,41 @@
> +* Atmel Quad Serial Peripheral Interface (QSPI)
> +
> +Required properties:
> +- compatible:     Should be "atmel,sama5d2-spi-qspi".
> +- reg:            Should contain the locations and lengths of the base registers
> +                  and the mapped memory.
> +- reg-names:      Should contain the resource reg names:
> +                  - qspi_base: configuration register address space
> +                  - qspi_mmap: memory mapped address space
> +- interrupts:     Should contain the interrupt for the device.
> +- clocks:         The phandle of the clock needed by the QSPI controller.
> +- #address-cells: Should be <1>.
> +- #size-cells:    Should be <0>.
> +
> +Example:
> +
> +qspi1: spi at f0024000 {
> +	compatible = "atmel,sama5d2-spi-qspi";
> +	reg = <0xf0024000 0x100>, <0xd8000000 0x08000000>;
> +	reg-names = "qspi_base", "qspi_mmap";
> +	interrupts = <53 IRQ_TYPE_LEVEL_HIGH 7>;
> +	clocks = <&qspi1_clk>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_qspi1_default>;
> +	status = "okay";
> +
> +	flash at 0 {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "winbond,w25m02gv", "spi-nand";

"winbond,w25m02gv" is undocumented and unnecessary since SPI NANDs are
automatically detected. Also, maybe you should declare a SPI NOR in the
example since SPI NAND support has not yet been merged.

> +		reg = <0>;
> +		spi-max-frequency = <83000000>;
> +		spi-rx-bus-width = <4>;
> +		spi-tx-bus-width = <4>;
> +
> +		...
> +	};
> +};
> +

Regards,

Boris

^ permalink raw reply

* [RFC PATCH 0/2] New QuadSPI driver for Atmel SAMA5D2
From: Boris Brezillon @ 2018-06-20 14:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180618162124.21749-1-bugalski.piotr@gmail.com>

Hi Piotr,

On Mon, 18 Jun 2018 18:21:22 +0200
Piotr Bugalski <bugalski.piotr@gmail.com> wrote:

> Hello,
> 
> Atmel SAMA5D2 is equipped with two QSPI interfaces. These interfaces can
> work as in SPI-compatible mode or use two / four lines to improve
> communication speed. At the moment there is QSPI driver strongly tied to
> NOR-flash memory and MTD subsystem.
> Intention of this change is to provide new driver which will not be tied
> to MTD and allows using QSPI with NAND-flash memory or other peripherals
> New spi-mem API provides abstraction layer which can disconnect QSPI
> from MTD. This driver doesn't support regular SPI interface, it should
> be used with spi-mem interface only.

Glad to see that people are starting to convert their SPI NOR
controller drivers to the SPI mem approach.

> Unfortunately SAMA5D2 hardware by default supports only NOR-flash
> memory. It allows 24- and 32-bit addressing while NAND-flash requires
> 16-bit long. To workaround hardware limitation driver is a bit more
> complicated.
> 
> Request to spi-mem contains three fiels: opcode (command), address,
> dummy bytes. SAMA5D2 QSPI hardware supports opcode, address, dummy and
> option byte where address field can only be 24- or 32- bytes long.
> Handling 8-bits long addresses is done using option field. For 16-bits
> address behaviour depends of number of requested dummy bits. If there
> are 8 or more dummy cycles, address is shifted and sent with first dummy
> byte. Otherwise opcode is disabled and first byte of address contains
> command opcode (works only if opcode and address use the same buswidth).
> The limitation is when 16-bit address is used without enough dummy
> cycles and opcode is using different buswidth than address. Other modes
> are supported with described workaround.
> 
> It looks like hardware has some limitation in performance. The same issue
> exists in current QSPI driver (MTD/nor-flash) and soft-pack (bare-metal
> library from Atmel). Without using DMA read speed is much worse than
> maximum bandwidth (efficiency 30-40%). Any help with performance
> improvement is highly welcome, especially for NAND-flash memories which
> offers higher capacity than NOR-flash used with previous driver.
> 
> Best Regards,
> Piotr
> 
> Piotr Bugalski (2):
>   spi: Add QuadSPI driver for Atmel SAMA5D2
>   dt-bindings: spi: QuadSPI driver for Atmel SAMA5D2 documentation
> 
>  .../devicetree/bindings/spi/spi_atmel-qspi.txt     |  41 ++
>  drivers/spi/Kconfig                                |   9 +
>  drivers/spi/Makefile                               |   1 +
>  drivers/spi/spi-atmel-qspi.c                       | 480 +++++++++++++++++++++

I'd like a solution where we remove the old driver. I definitely don't
want to have both in parallel. Did you test the new driver with a SPI
NOR to check if it still works correctly? If you did, then I'd suggest
that you add a patch updating defconfigs where the SPI_ATMEL_QUADSPI is
selected and another patch removing the old driver.

>  4 files changed, 531 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi_atmel-qspi.txt

This should be a simple mv from
Documentation/devicetree/bindings/mtd/atmel-quadspi.txt to
Documentation/devicetree/bindings/spi/spi-atmel-qspi.txt

>  create mode 100644 drivers/spi/spi-atmel-qspi.c
> 

Thanks,

Boris

^ permalink raw reply

* [PATCH 5/6] dmaengine: xilinx_dma: autodetect whether the HW supports scatter-gather
From: Andrea Merello @ 2018-06-20 14:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <DM6PR02MB436181CD5544B0FA95318077C7770@DM6PR02MB4361.namprd02.prod.outlook.com>

On Wed, Jun 20, 2018 at 4:43 PM, Radhey Shyam Pandey <radheys@xilinx.com> wrote:
>> -----Original Message-----
>> From: dmaengine-owner at vger.kernel.org [mailto:dmaengine-
>> owner at vger.kernel.org] On Behalf Of Andrea Merello
>> Sent: Wednesday, June 20, 2018 2:07 PM
>> To: vkoul at kernel.org; dan.j.williams at intel.com; Michal Simek
>> <michals@xilinx.com>; Appana Durga Kedareswara Rao
>> <appanad@xilinx.com>; dmaengine at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org;
>> Andrea Merello <andrea.merello@gmail.com>
>> Subject: [PATCH 5/6] dmaengine: xilinx_dma: autodetect whether the HW
>> supports scatter-gather
>>
>> The HW can be either direct-access or scatter-gather version. These are
>> SW incompatible.
>>
>> The driver can handle both version: a DT property was used to
>> tell the driver whether to assume the HW is is scatter-gather mode.
>>
>> This patch makes the driver to autodetect this information. The DT
>> property is not required anymore.
>>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> ---
>>  drivers/dma/xilinx/xilinx_dma.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
>> index bdbc8ba9092a..8c6e818e596f 100644
>> --- a/drivers/dma/xilinx/xilinx_dma.c
>> +++ b/drivers/dma/xilinx/xilinx_dma.c
>> @@ -86,6 +86,7 @@
>>  #define XILINX_DMA_DMASR_DMA_DEC_ERR         BIT(6)
>>  #define XILINX_DMA_DMASR_DMA_SLAVE_ERR               BIT(5)
>>  #define XILINX_DMA_DMASR_DMA_INT_ERR         BIT(4)
>> +#define XILINX_DMA_DMASR_SG_MASK             BIT(3)
>>  #define XILINX_DMA_DMASR_IDLE                        BIT(1)
>>  #define XILINX_DMA_DMASR_HALTED              BIT(0)
>>  #define XILINX_DMA_DMASR_DELAY_MASK          GENMASK(31, 24)
>> @@ -407,7 +408,6 @@ struct xilinx_dma_config {
>>   * @dev: Device Structure
>>   * @common: DMA device structure
>>   * @chan: Driver specific DMA channel
>> - * @has_sg: Specifies whether Scatter-Gather is present or not
>>   * @mcdma: Specifies whether Multi-Channel is present or not
>>   * @flush_on_fsync: Flush on frame sync
>>   * @ext_addr: Indicates 64 bit addressing is supported by dma device
>> @@ -426,7 +426,6 @@ struct xilinx_dma_device {
>>       struct device *dev;
>>       struct dma_device common;
>>       struct xilinx_dma_chan
>> *chan[XILINX_DMA_MAX_CHANS_PER_DEVICE];
>> -     bool has_sg;
>>       bool mcdma;
>>       u32 flush_on_fsync;
>>       bool ext_addr;
>> @@ -2391,7 +2390,6 @@ static int xilinx_dma_chan_probe(struct
>> xilinx_dma_device *xdev,
>>
>>       chan->dev = xdev->dev;
>>       chan->xdev = xdev;
>> -     chan->has_sg = xdev->has_sg;
>>       chan->desc_pendingcount = 0x0;
>>       chan->ext_addr = xdev->ext_addr;
>>       /* This variable ensures that descriptors are not
>> @@ -2488,6 +2486,13 @@ static int xilinx_dma_chan_probe(struct
>> xilinx_dma_device *xdev,
>>               chan->stop_transfer = xilinx_dma_stop_transfer;
>>       }
>>
>> +     /* check if SG is enabled */
>> +     if (dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
>> +             XILINX_DMA_DMASR_SG_MASK)
> I think SGIncld mask is only applicable for AXI DMA and CDMA IP.
> For VDMA IP this bit is reserved.

OK. I can make it conditional wrt the IP type. As far as I can see
VDMA IP has not the two (SG vs no-SG) variant at all, so all should be
still OK.

> .
>> +             chan->has_sg = true;
>> +     dev_dbg(chan->dev, "ch %d: SG %s\n", chan->id,
>> +             chan->has_sg ? "enabled" : "disabled");
>> +
>>       /* Initialize the tasklet */
>>       tasklet_init(&chan->tasklet, xilinx_dma_do_tasklet,
>>                       (unsigned long)chan);
>> @@ -2626,7 +2631,6 @@ static int xilinx_dma_probe(struct platform_device
>> *pdev)
>>               return PTR_ERR(xdev->regs);
>>
>>       /* Retrieve the DMA engine properties from the device tree */
>> -
> Unrelated change

Oops.. Sorry.

>
>>       if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
>>               xdev->mcdma = of_property_read_bool(node, "xlnx,mcdma");
>>               err = of_property_read_u32(node, "xlnx,lengthregwidth",
>> --
>> 2.17.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


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