Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* arm64: renesas: r8a7796/salvator-x: Add board part number to DT bindings
From: Simon Horman @ 2016-11-30 10:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1472635054-24372-1-git-send-email-geert+renesas@glider.be>

On Wed, Aug 31, 2016 at 11:17:34AM +0200, Geert Uytterhoeven wrote:
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks, queued up for v4.11.

^ permalink raw reply

* [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
From: Ganapatrao Kulkarni @ 2016-11-30 10:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6c9012fd-070b-6218-48e7-69b37f2559dd@redhat.com>

Hi Eric,

in you repo "https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3"
there is 11th patch "pci: Enable overrides for missing ACS capabilities"
is this patch part of some other series?

thanks
Ganapat

On Wed, Nov 30, 2016 at 3:19 PM, Auger Eric <eric.auger@redhat.com> wrote:
> Hi,
>
> On 15/11/2016 14:09, Eric Auger wrote:
>> Following LPC discussions, we now report reserved regions through
>> iommu-group sysfs reserved_regions attribute file.
>>
>> Reserved regions are populated through the IOMMU get_resv_region callback
>> (former get_dm_regions), now implemented by amd-iommu, intel-iommu and
>> arm-smmu.
>>
>> The intel-iommu reports the [FEE0_0000h - FEF0_000h] MSI window as an
>> IOMMU_RESV_NOMAP reserved region.
>>
>> arm-smmu reports the MSI window (arbitrarily located at 0x8000000 and
>> 1MB large) and the PCI host bridge windows.
>>
>> The series integrates a not officially posted patch from Robin:
>> "iommu/dma: Allow MSI-only cookies".
>>
>> This series currently does not address IRQ safety assessment.
>
> I will respin this series taking into account Joerg's comment. Does
> anyone have additional comments or want to put forward some conceptual
> issues with the current direction and with this implementation?
>
> As for the IRQ safety assessment, in a first step I would propose to
> remove the IOMMU_CAP_INTR_REMAP from arm-smmus and consider the
> assignment as unsafe. Any objection?
>
> Thanks
>
> Eric
>
>
>> Best Regards
>>
>> Eric
>>
>> Git: complete series available at
>> https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3
>>
>> History:
>> RFC v2 -> v3:
>> - switch to an iommu-group sysfs API
>> - use new dummy allocator provided by Robin
>> - dummy allocator initialized by vfio-iommu-type1 after enumerating
>>   the reserved regions
>> - at the moment ARM MSI base address/size is left unchanged compared
>>   to v2
>> - we currently report reserved regions and not usable IOVA regions as
>>   requested by Alex
>>
>> RFC v1 -> v2:
>> - fix intel_add_reserved_regions
>> - add mutex lock/unlock in vfio_iommu_type1
>>
>>
>> Eric Auger (10):
>>   iommu/dma: Allow MSI-only cookies
>>   iommu: Rename iommu_dm_regions into iommu_resv_regions
>>   iommu: Add new reserved IOMMU attributes
>>   iommu: iommu_alloc_resv_region
>>   iommu: Do not map reserved regions
>>   iommu: iommu_get_group_resv_regions
>>   iommu: Implement reserved_regions iommu-group sysfs file
>>   iommu/vt-d: Implement reserved region get/put callbacks
>>   iommu/arm-smmu: Implement reserved region get/put callbacks
>>   vfio/type1: Get MSI cookie
>>
>>  drivers/iommu/amd_iommu.c       |  20 +++---
>>  drivers/iommu/arm-smmu.c        |  52 +++++++++++++++
>>  drivers/iommu/dma-iommu.c       | 116 ++++++++++++++++++++++++++-------
>>  drivers/iommu/intel-iommu.c     |  50 ++++++++++----
>>  drivers/iommu/iommu.c           | 141 ++++++++++++++++++++++++++++++++++++----
>>  drivers/vfio/vfio_iommu_type1.c |  26 ++++++++
>>  include/linux/dma-iommu.h       |   7 ++
>>  include/linux/iommu.h           |  49 ++++++++++----
>>  8 files changed, 391 insertions(+), 70 deletions(-)
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] arm64: dts: juno: Correct PCI IO window
From: liviu.dudau at arm.com @ 2016-11-30 10:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480452310-29286-1-git-send-email-jeremy.linton@arm.com>

On Tue, Nov 29, 2016 at 02:45:10PM -0600, Jeremy Linton wrote:
> The PCIe root complex on Juno translates the MMIO mapped
> at 0x5f800000 to the PIO address range starting at 0
> (which is common because PIO addresses are generally < 64k).
> Correct the DT to reflect this.
> 
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>

With the U-Boot patch that I have sent to the ML:
Tested-by: Liviu Dudau <Liviu.Dudau@arm.com>

also

Acked-by: Liviu Dudau <Liviu.Dudau@arm.com>

Best regards,
Liviu


> ---
>  arch/arm64/boot/dts/arm/juno-base.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi
> index 334271a..7d3a2ac 100644
> --- a/arch/arm64/boot/dts/arm/juno-base.dtsi
> +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi
> @@ -393,7 +393,7 @@
>  		#address-cells = <3>;
>  		#size-cells = <2>;
>  		dma-coherent;
> -		ranges = <0x01000000 0x00 0x5f800000 0x00 0x5f800000 0x0 0x00800000>,
> +		ranges = <0x01000000 0x00 0x00000000 0x00 0x5f800000 0x0 0x00800000>,
>  			 <0x02000000 0x00 0x50000000 0x00 0x50000000 0x0 0x08000000>,
>  			 <0x42000000 0x40 0x00000000 0x40 0x00000000 0x1 0x00000000>;
>  		#interrupt-cells = <1>;
> -- 
> 2.5.5
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

^ permalink raw reply

* [RFC v2: PATCH 1/2] dt-bindings: Document the hi3660 reset bindings
From: Philipp Zabel @ 2016-11-30 10:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <bec21998-b30c-4076-62bf-6b24c9dd1dd6@linaro.org>

Am Freitag, den 25.11.2016, 20:08 +0800 schrieb zhangfei:
> 
> On 2016?11?25? 18:54, Philipp Zabel wrote:
> > Am Freitag, den 25.11.2016, 18:42 +0800 schrieb zhangfei:
> >> On 2016?11?25? 18:25, Philipp Zabel wrote:
> >>> Am Donnerstag, den 24.11.2016, 18:20 +0800 schrieb zhangfei:
> >>>> On 2016?11?24? 17:50, Philipp Zabel wrote:
> >>>>> Am Donnerstag, den 24.11.2016, 17:40 +0800 schrieb zhangfei:
> >>>>>> On 2016?11?24? 17:26, Philipp Zabel wrote:
> >>>>>>> Am Mittwoch, den 23.11.2016, 16:07 +0800 schrieb Zhangfei Gao:
> >>>>>>>> Add DT bindings documentation for hi3660 SoC reset controller.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> >>>>>>>> ---
> >>>>>>>>      .../bindings/reset/hisilicon,hi3660-reset.txt      | 51 ++++++++++++++++++++++
> >>>>>>>>      1 file changed, 51 insertions(+)
> >>>>>>>>      create mode 100644 Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>>
> >>>>>>>> diff --git a/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>> new file mode 100644
> >>>>>>>> index 0000000..250daf2
> >>>>>>>> --- /dev/null
> >>>>>>>> +++ b/Documentation/devicetree/bindings/reset/hisilicon,hi3660-reset.txt
> >>>>>>>> @@ -0,0 +1,51 @@
> >>>>>>>> +Hisilicon System Reset Controller
> >>>>>>>> +======================================
> >>>>>>>> +
> >>>>>>>> +Please also refer to reset.txt in this directory for common reset
> >>>>>>>> +controller binding usage.
> >>>>>>>> +
> >>>>>>>> +The reset controller registers are part of the system-ctl block on
> >>>>>>>> +hi3660 SoC.
> >>>>>>>> +
> >>>>>>>> +Required properties:
> >>>>>>>> +- compatible: should be
> >>>>>>>> +		 "hisilicon,hi3660-reset"
> >>>>>>>> +- #reset-cells: 1, see below
> >>>>>>>> +- hisi,rst-syscon: phandle of the reset's syscon.
> >>>>>>>> +- hisi,reset-bits: Contains the reset control register information
> >>>>>>>> +		  Should contain 2 cells for each reset exposed to
> >>>>>>>> +		  consumers, defined as:
> >>>>>>>> +			Cell #1 : offset from the syscon register base
> >>>>>>>> +			Cell #2 : bits position of the control register
> >>>>>>>> +
> >>>>>>>> +Example:
> >>>>>>>> +	iomcu: iomcu at ffd7e000 {
> >>>>>>>> +		compatible = "hisilicon,hi3660-iomcu", "syscon";
> >>>>>>>> +		reg = <0x0 0xffd7e000 0x0 0x1000>;
> >>>>>>>> +	};
> >>>>>>>> +
> >>>>>>>> +	iomcu_rst: iomcu_rst_controller {
> >>>>>>> This should be
> >>>>>>> 	iomcu_rst: reset-controller {
> >>>>>>>
> >>>>>>>> +		compatible = "hisilicon,hi3660-reset";
> >>>>>>>> +		#reset-cells = <1>;
> >>>>>>>> +		hisi,rst-syscon = <&iomcu>;
> >>>>>>>> +		hisi,reset-bits = <0x20 0x8		/* 0: i2c0 */
> >>>>>>>> +				   0x20 0x10		/* 1: i2c1 */
> >>>>>>>> +				   0x20 0x20		/* 2: i2c2 */
> >>>>>>>> +				   0x20 0x8000000>;	/* 3: i2c6 */
> >>>>>>>> +	};
> >>>>>>> The reset lines are controlled through iomcu bits, is there a reason not
> >>>>>>> to put the iomcu_rst node inside the iomcu node? That way the
> >>>>>>> hisi,rst-syscon property could be removed and the syscon could be
> >>>>>>> retrieved via the reset-controller parent node.
> >>>>>> iomcu is common registers, controls clock and reset, etc.
> >>>>>> So we use syscon, without mapping the registers everywhere.
> >>>>>> It is common case in hisilicon, same in hi6220.
> >>>>>>
> >>>>>> Also the #clock-cells and #reset-cells can not be put in the same node,
> >>>>>> if they are both using probe, since reset_probe will not be called.
> >>>>>>
> >>>>>> So we use hisi,rst-syscon as a general solution.
> >>>>> What I meant is this:
> >>>>>
> >>>>> 	iomcu: iomcu at ffd7e000 {
> >>>>> 		compatible = "hisilicon,hi3660-iomcu", "syscon", "simple-mfd";
> >>>>> 		reg = <0x0 0xffd7e000 0x0 0x1000>;
> >>>> #clock-cells = <1>;
> >>>>
> >>>> In my test, if there add #clock-cells = <1>, reset_probe will not be
> >>>> called any more.
> >>>> Since clk_probe is called first.
> >>>> No matter iomcu_rst is child node or not.
> >>> I don't understand this, does the clock driver bind to the iomcu node
> >>> using CLK_OF_DECLARE_DRIVER(..., "hisilicon,hi3660-iomcu", ...)?
> >> This method:CLK_OF_DECLARE_DRIVER is not prefered in clock,
> >> and we have to use probe instead, to make all driver build as modules as
> >> possible.
> >>
> >> For example hi3660.
> >> static struct platform_driver hi3660_clk_driver = {
> >>           .probe          = hi3660_clk_probe,
> >>           .driver         = {
> >>                   .name   = "hi3660-clk",
> >>                   .of_match_table = hi3660_clk_match_table,
> >>           },
> >> };
> > hi3660_clk_match_table contains the "hisilicon,hi3660-iomcu" compatible?
> > If so, you could call
> > 	of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> > from hi3660_clk_probe instead of using "simple-mfd" to probe the iomcu
> > node's children.
> 
> Not using simple-mfd:
> 
> Like
> static const struct of_device_id hi3660_clk_match_table[] = {
>          { .compatible = "hisilicon,hi3660-iomcu", },
>          { }
> };
> MODULE_DEVICE_TABLE(of, hi3660_clk_match_table);
> 
> static int hi3660_clk_probe(struct platform_device *pdev)
> {
>          struct device *dev = &pdev->dev;
>          struct device_node *np = pdev->dev.of_node;
>          const struct of_device_id *of_id;
>          enum hi3660_clk_type type;
> 
>          of_id = of_match_device(hi3660_clk_match_table, dev);
>          if (!of_id)
>                  return -EINVAL;
> ~
> }
> 
> If put iomcu_rst as child node, and set #clock-cells = <1> to iomcu,
> then hi3660_clk_probe is called, hi3660_reset_probe will not be called.

For hi3660_reset_probe to be called, you'll have to call
of_platform_populate to probe the hi3660-iomcu children in this case.

> So using "hisi,rst-syscon" as pointer does not have the issue.

I understand that, it still sounds to me like you are organizing the
device tree around limitations of the current code. Instead the device
tree should be organized to best describe the hardware, and the code
should be adapted to support that.

Of course, if you use the flat DT layout everywhere else, I won't try to
block the reset driver because of this issue. I'm just saying nested
nodes in the DT would better describe the real control flow.

regards
Philipp

^ permalink raw reply

* [RFC v2 PATCH 23/23] ARM: Allow ARCH_MULTIPLATFORM to be selected for NOMMU
From: Vladimir Murzin @ 2016-11-30 10:01 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161129172234.GY14217@n2100.armlinux.org.uk>

On 29/11/16 17:22, Russell King - ARM Linux wrote:
> On Tue, Nov 29, 2016 at 12:40:05PM +0000, Vladimir Murzin wrote:
>> With this patch applied potentially any platform can be built in NOMMU
>> configurations if CONFIG_EXPERT is selected. However, there is no
>> guaranty that platform can successfully run such Image. So the main
> 
> guarantee
> 
>> motivation behind of this patch:
>> - bring build coverage for NOMMU configurations
>> - allow known working NOMMU platforms (like R-class) to be used
>> - pave a way to add support for single address space (aka 1:1 mapping)
>>   for MMU platforms, so they can be usable in NOMMU configurations
>>
>> Cc: Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: Ryan Mallon <rmallon@gmail.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: Alexander Shiyan <shc_work@mail.ru>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: Sascha Hauer <kernel@pengutronix.de>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>>  arch/arm/Kconfig |   21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index e78c822..bc6f406 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -327,9 +327,9 @@ choice
>>  
>>  config ARCH_MULTIPLATFORM
>>  	bool "Allow multiple platforms to be selected"
>> -	depends on MMU
>> +	depends on MMU || EXPERT
>>  	select ARM_HAS_SG_CHAIN
>> -	select ARM_PATCH_PHYS_VIRT
>> +	select ARM_PATCH_PHYS_VIRT if MMU
>>  	select AUTO_ZRELADDR
>>  	select CLKSRC_OF
>>  	select COMMON_CLK
>> @@ -339,6 +339,23 @@ config ARCH_MULTIPLATFORM
>>  	select PCI_DOMAINS if PCI
>>  	select SPARSE_IRQ
>>  	select USE_OF
>> +	help
>> +	  Please, read carefully if you've selected CONFIG_MMU=n!
>> +
>> +	  Multiplatform with !MMU configuration *is not* meant that
>> +	  kernel built to support every platform will boot on them. It
>> +	  is because physical address space layouts (particularly where
>> +	  RAM is located) are different between platforms and there is
>> +	  no MMU to work that around.
>> +
>> +	  You must specify where RAM start (via DRAM_BASE config
>> +	  option) and appropriate size of RAM (via DRAM_SIZE config
>> +	  option) which are valid for the platform you are building
>> +	  for.
>> +
>> +	  This feature is *EXPERIMENTAL*, please, consider building
>> +	  with CONFIG_MMU=y unless you know what you do or want to
>> +	  help with testing.
> 
> Do you actually see this help text anywhere?  I don't think multiple-choice
> options show help for individual choices.
> 

Yes I can: "System Type" -> "ARM system type" -> "Allow multiple platforms to
be selected" -> "?". Probably not the best place, but I failed to find
anything better :(

Cheers
Vladimir

^ permalink raw reply

* [RFC v2 PATCH 08/23] ARM: NOMMU: implement secondary_startup_arm
From: Vladimir Murzin @ 2016-11-30  9:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161129171432.GX14217@n2100.armlinux.org.uk>

On 29/11/16 17:14, Russell King - ARM Linux wrote:
> On Tue, Nov 29, 2016 at 12:39:50PM +0000, Vladimir Murzin wrote:
>> Mediatek's and Qualcomm's platform code has reference to
>> secondary_startup_arm and that breaks NOMMU build.
> 
> This needs to explain why this is safe in the presence of EFM32.
> 

It did not explode by pure luck M-class doesn't support SMP. The code needs to
include case for CONFIG_CPU_THUMBONLY. I'll include update in the next version.

Thanks
Vladimir

>> Cc: Russell King <linux@armlinux.org.uk>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
>> ---
>>  arch/arm/kernel/head-nommu.S |    7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
>> index 7317554..2ab026f 100644
>> --- a/arch/arm/kernel/head-nommu.S
>> +++ b/arch/arm/kernel/head-nommu.S
>> @@ -89,6 +89,12 @@ ENDPROC(stext)
>>  
>>  #ifdef CONFIG_SMP
>>  	.text
>> +	.arm
>> +ENTRY(secondary_startup_arm)
>> +	THUMB(	badr	r9, 1f		)	@ Kernel is entered in ARM.
>> +	THUMB(	bx	r9		)	@ If this is a Thumb-2 kernel,
>> +	THUMB(	.thumb			)	@ switch to Thumb now.
>> +	THUMB(1:			)
>>  ENTRY(secondary_startup)
>>  	/*
>>  	 * Common entry point for secondary CPUs.
>> @@ -126,6 +132,7 @@ ENTRY(secondary_startup)
>>  	mov	fp, #0
>>  	b	secondary_start_kernel
>>  ENDPROC(secondary_startup)
>> +ENDPROC(secondary_startup_arm)
>>  
>>  	.type	__secondary_data, %object
>>  __secondary_data:
>> -- 
>> 1.7.9.5
>>
> 

^ permalink raw reply

* [RFC PATCH 00/29] arm64: Scalable Vector Extension core support
From: Yao Qi @ 2016-11-30  9:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1480102762-23647-1-git-send-email-Dave.Martin@arm.com>

Hi, Dave,

On Fri, Nov 25, 2016 at 7:38 PM, Dave Martin <Dave.Martin@arm.com> wrote:
>  * No independent SVE vector length configuration per thread.  This is
>    planned, but will follow as a separate add-on series.

If I read "independent SVE vector length configuration per thread"
correctly, SVE vector length can be different in each thread, so the
size of vector registers is different too.  In GDB, we describe registers
by "target description" which is per process, not per thread.

-- 
Yao (??)

^ permalink raw reply

* [RFC v2 PATCH 11/23] ARM: sleep: allow it to be build for R-class
From: Vladimir Murzin @ 2016-11-30  9:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161129171145.GW14217@n2100.armlinux.org.uk>

On 29/11/16 17:11, Russell King - ARM Linux wrote:
> On Tue, Nov 29, 2016 at 12:39:53PM +0000, Vladimir Murzin wrote:
>> Dependency on MMU is quite strict and prevent R-class from being built -
>> relax this condition and guard against M-class only
> 
> I thought I'd already commented on some of this change, but it seems
> not.
> 
>> diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S
>> index 0f6c100..0e7fddf 100644
>> --- a/arch/arm/kernel/sleep.S
>> +++ b/arch/arm/kernel/sleep.S
>> @@ -119,14 +119,12 @@ ENDPROC(cpu_resume_after_mmu)
>>  	.text
>>  	.align
>>  
>> -#ifdef CONFIG_MMU
>>  	.arm
>>  ENTRY(cpu_resume_arm)
>>   THUMB(	badr	r9, 1f		)	@ Kernel is entered in ARM.
>>   THUMB(	bx	r9		)	@ If this is a Thumb-2 kernel,
>>   THUMB(	.thumb			)	@ switch to Thumb now.
>>   THUMB(1:			)
>> -#endif
>>  
>>  ENTRY(cpu_resume)
>>  ARM_BE8(setend be)			@ ensure we are in BE mode
>> @@ -160,9 +158,7 @@ THUMB(	mov	sp, r2			)
>>  THUMB(	bx	r3			)
>>  ENDPROC(cpu_resume)
>>  
>> -#ifdef CONFIG_MMU
>>  ENDPROC(cpu_resume_arm)
>> -#endif
> 
> These ifdefs were introduced to fix EFM32.  The commit description needs
> to state why it's safe to remove them now (presumably because of the
> dependency on !CPU_V7M).  However, it also needs to explain why it's
> not going to cause a regression for EFM32 - EFM32 _was_ capable of
> building this code.
> 

After looking at 2678bb9f ("ARM: fix EFM32 build breakage caused by
cpu_resume_arm") I think the right fix should be based on CONFIG_CPU_THUMBONLY
rather than disallowing this code to be build for CPU_V7M. 

Indeed, I've just tried that it it works fine, so I'll update the patch to
reflect that.

Thanks for your valuable comment!

Cheers
Vladimir

^ permalink raw reply

* [PATCH] arm64: defconfig: Enable NUMA and NUMA_BALANCING
From: Kefeng Wang @ 2016-11-30  9:52 UTC (permalink / raw)
  To: linux-arm-kernel

Enable NUMA and NUMA_BALANCING to improve the performance on board
with multiple numa nodes.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index dab2cb0..967f6e5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -12,6 +12,7 @@ CONFIG_TASK_IO_ACCOUNTING=y
 CONFIG_IKCONFIG=y
 CONFIG_IKCONFIG_PROC=y
 CONFIG_LOG_BUF_SHIFT=14
+CONFIG_NUMA_BALANCING=y
 CONFIG_MEMCG=y
 CONFIG_MEMCG_SWAP=y
 CONFIG_BLK_CGROUP=y
@@ -72,6 +73,7 @@ CONFIG_PCIE_QCOM=y
 CONFIG_PCIE_ARMADA_8K=y
 CONFIG_ARM64_VA_BITS_48=y
 CONFIG_SCHED_MC=y
+CONFIG_NUMA=y
 CONFIG_PREEMPT=y
 CONFIG_KSM=y
 CONFIG_TRANSPARENT_HUGEPAGE=y
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Laurent Pinchart @ 2016-11-30  9:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161130102757.9eec1f7f3377d0f4787e3829@free.fr>

Hi Jean-Fran?ois,

On Wednesday 30 Nov 2016 10:27:57 Jean-Francois Moine wrote:
> On Wed, 30 Nov 2016 10:20:21 +0200 Laurent Pinchart wrote:
> >> Well, I don't see what this connector can be.
> >> May you give me a DT example?
> > 
> > Sure.
> > 
> > arch/arm/boot/dts/r8a7791-koelsch.dts
> > 
> >         /* HDMI encoder */
> >         
> >         hdmi at 39 {
> >                 compatible = "adi,adv7511w";
> >                 reg = <0x39>;
> >                 interrupt-parent = <&gpio3>;
> >                 interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> >                 
> >                 adi,input-depth = <8>;
> >                 adi,input-colorspace = "rgb";
> >                 adi,input-clock = "1x";
> >                 adi,input-style = <1>;
> >                 adi,input-justification = "evenly";
> >                 
> >                 ports {
> >                         #address-cells = <1>;
> >                         #size-cells = <0>;
> >                         
> >                         port at 0 {
> >                                 reg = <0>;
> >                                 adv7511_in: endpoint {
> >                                         remote-endpoint = <&du_out_rgb>;
> >                                 };
> >                         };
> >                         
> >                         port at 1 {
> >                                 reg = <1>;
> >                                 adv7511_out: endpoint {
> >                                         remote-endpoint = <&hdmi_con>;
> >                                 };
> >                         };
> >                 };
> >         
> >         };
> >         
> >         /* HDMI connector */
> >         
> >         hdmi-out {
> >                 compatible = "hdmi-connector";
> >                 type = "a";
> >                 
> >                 port {
> >                         hdmi_con: endpoint {
> >                                 remote-endpoint = <&adv7511_out>;
> >                         };
> >                 };
> >         };
> 
> Hi Laurent,
> 
> Sorry for I don't see the interest:
> - it is obvious that the HDMI connector is a 'hdmi-connector'!

It still has to be told to the drivers, they don't know how to identify a 
connector by looking at it :-)

> - the physical connector type may be changed on any board by a soldering
>   iron or a connector to connector cable.

Which is also true for any other component on the board. DT (and for that 
matter any firmware description of the platform) isn't soldering-proof.

> - what does the software do with the connector type?

That's up to the software to decide, the DT bindings should describe the 
hardware in the most accurate and usable way for the OS as possible. One of my 
longer term goals is to add connector drivers to handle DDC and HPD when 
they're not handled by the encoder (they are in the above example).

If the DDC was connected to a general-purpose I2C bus of the SoC, and the HPD 
to a GPIO, we would have

	hdmi-out {
		compatible = "hdmi-connector";
		type = "a";
		/* I2C bus and GPIO references are made up for the example */
		ddc-i2c-bus = <&i2c4>;
		hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>

		port {
			hdmi_con: endpoint {
				remote-endpoint = <&adv7511_out>;
			};
		};
	};

and both HPD and EDID reading should be handled by the connector driver.

> - why not to put the connector information in the HDMI device?

Because the connector is separate from the encoder. It's not uncommon 
(depending on the encoder type) to have the encoder output connected to a non-
connector entity such as another chained encoder.

For example most LVDS encoders are connected to a panel, but I have a board 
with the following encoders chain.

CRTC -- parallel RGB --> on-SoC LVDS encoder -- LVDS --> on-board LVDS decoder 
-- parallel RGB --> HDMI encoder -- HDMI --> HDMI connector

I can't support that if the LVDS encoder driver hardcodes the assumption that 
the encoder output is connected to a panel. This kind of usage might be less 
common for HDMI but is certainly not inconceivable.

> And, if I follow you, the graph of ports could also be used to describe
> the way the various parts of the SoCs are powered, to describe the pin
> connections, to describe the USB connectors, to describe the board
> internal hubs and bridges...

It should be used where applicable, it's not meant as the only possible 
hardware description for all pieces of the system.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [PATCH v2] arm64: dts: zx: add zx296718's topcrm node
From: Baoyou Xie @ 2016-11-30  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

Enable topcrm clock node for zx296718, which is used for
CPU's frequency change.

Furthermore, this patch adds the CPU clock phandle in CPU's node
and uses operating-points-v2 to register operating points.

So it can be used by cpufreq-dt driver.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 arch/arm64/boot/dts/zte/zx296718.dtsi | 43 +++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index 6b239a3..992158a 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -44,6 +44,7 @@
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/clock/zx296718-clock.h>
 
 / {
 	compatible = "zte,zx296718";
@@ -81,6 +82,8 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x0>;
 			enable-method = "psci";
+			clocks = <&topcrm A53_GATE>;
+			operating-points-v2 = <&cluster0_opp>;
 		};
 
 		cpu1: cpu at 1 {
@@ -88,6 +91,7 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x1>;
 			enable-method = "psci";
+			operating-points-v2 = <&cluster0_opp>;
 		};
 
 		cpu2: cpu at 2 {
@@ -95,6 +99,7 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x2>;
 			enable-method = "psci";
+			operating-points-v2 = <&cluster0_opp>;
 		};
 
 		cpu3: cpu at 3 {
@@ -102,6 +107,38 @@
 			compatible = "arm,cortex-a53","arm,armv8";
 			reg = <0x0 0x3>;
 			enable-method = "psci";
+			operating-points-v2 = <&cluster0_opp>;
+		};
+	};
+
+	cluster0_opp: opp_table0 {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp at 500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+			opp-microvolt = <857000>;
+			clock-latency-ns = <500000>;
+		};
+		opp at 648000000 {
+			opp-hz = /bits/ 64 <648000000>;
+			opp-microvolt = <857000>;
+			clock-latency-ns = <500000>;
+		};
+		opp at 800000000 {
+			opp-hz = /bits/ 64 <800000000>;
+			opp-microvolt = <882000>;
+			clock-latency-ns = <500000>;
+		};
+		opp at 1000000000 {
+			opp-hz = /bits/ 64 <1000000000>;
+			opp-microvolt = <892000>;
+			clock-latency-ns = <500000>;
+		};
+		opp at 1188000000 {
+			opp-hz = /bits/ 64 <1188000000>;
+			opp-microvolt = <1009000>;
+			clock-latency-ns = <500000>;
 		};
 	};
 
@@ -279,6 +316,12 @@
 			dma-requests = <32>;
 		};
 
+		topcrm: clock-controller at 1461000 {
+			compatible = "zte,zx296718-topcrm";
+			reg = <0x01461000 0x1000>;
+			#clock-cells = <1>;
+		};
+
 		sysctrl: sysctrl at 1463000 {
 			compatible = "zte,zx296718-sysctrl", "syscon";
 			reg = <0x1463000 0x1000>;
-- 
2.7.4

^ permalink raw reply related

* [RFC v3 00/10] KVM PCIe/MSI passthrough on ARM/ARM64 and IOVA reserved regions
From: Auger Eric @ 2016-11-30  9:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479215363-2898-1-git-send-email-eric.auger@redhat.com>

Hi,

On 15/11/2016 14:09, Eric Auger wrote:
> Following LPC discussions, we now report reserved regions through
> iommu-group sysfs reserved_regions attribute file.
> 
> Reserved regions are populated through the IOMMU get_resv_region callback
> (former get_dm_regions), now implemented by amd-iommu, intel-iommu and
> arm-smmu.
> 
> The intel-iommu reports the [FEE0_0000h - FEF0_000h] MSI window as an
> IOMMU_RESV_NOMAP reserved region.
> 
> arm-smmu reports the MSI window (arbitrarily located at 0x8000000 and
> 1MB large) and the PCI host bridge windows.
> 
> The series integrates a not officially posted patch from Robin:
> "iommu/dma: Allow MSI-only cookies".
> 
> This series currently does not address IRQ safety assessment.

I will respin this series taking into account Joerg's comment. Does
anyone have additional comments or want to put forward some conceptual
issues with the current direction and with this implementation?

As for the IRQ safety assessment, in a first step I would propose to
remove the IOMMU_CAP_INTR_REMAP from arm-smmus and consider the
assignment as unsafe. Any objection?

Thanks

Eric


> Best Regards
> 
> Eric
> 
> Git: complete series available at
> https://github.com/eauger/linux/tree/v4.9-rc5-reserved-rfc-v3
> 
> History:
> RFC v2 -> v3:
> - switch to an iommu-group sysfs API
> - use new dummy allocator provided by Robin
> - dummy allocator initialized by vfio-iommu-type1 after enumerating
>   the reserved regions
> - at the moment ARM MSI base address/size is left unchanged compared
>   to v2
> - we currently report reserved regions and not usable IOVA regions as
>   requested by Alex
> 
> RFC v1 -> v2:
> - fix intel_add_reserved_regions
> - add mutex lock/unlock in vfio_iommu_type1
> 
> 
> Eric Auger (10):
>   iommu/dma: Allow MSI-only cookies
>   iommu: Rename iommu_dm_regions into iommu_resv_regions
>   iommu: Add new reserved IOMMU attributes
>   iommu: iommu_alloc_resv_region
>   iommu: Do not map reserved regions
>   iommu: iommu_get_group_resv_regions
>   iommu: Implement reserved_regions iommu-group sysfs file
>   iommu/vt-d: Implement reserved region get/put callbacks
>   iommu/arm-smmu: Implement reserved region get/put callbacks
>   vfio/type1: Get MSI cookie
> 
>  drivers/iommu/amd_iommu.c       |  20 +++---
>  drivers/iommu/arm-smmu.c        |  52 +++++++++++++++
>  drivers/iommu/dma-iommu.c       | 116 ++++++++++++++++++++++++++-------
>  drivers/iommu/intel-iommu.c     |  50 ++++++++++----
>  drivers/iommu/iommu.c           | 141 ++++++++++++++++++++++++++++++++++++----
>  drivers/vfio/vfio_iommu_type1.c |  26 ++++++++
>  include/linux/dma-iommu.h       |   7 ++
>  include/linux/iommu.h           |  49 ++++++++++----
>  8 files changed, 391 insertions(+), 70 deletions(-)
> 

^ permalink raw reply

* [PATCH net-next v4 0/4] Fix OdroidC2 Gigabit Tx link issue
From: Jerome Brunet @ 2016-11-30  9:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <049b1efc-3bad-92e0-45ef-0563dc5d81de@gmail.com>

On Mon, 2016-11-28 at 09:54 -0800, Florian Fainelli wrote:
> On 11/28/2016 07:50 AM, Jerome Brunet wrote:
> > 
> > This patchset fixes an issue with the OdroidC2 board (DWMAC +
> > RTL8211F).
> > The platform seems to enter LPI on the Rx path too often while
> > performing
> > relatively high TX transfer. This eventually break the link (both
> > Tx and
> > Rx), and require to bring the interface down and up again to get
> > the Rx
> > path working again.
> > 
> > The root cause of this issue is not fully understood yet but
> > disabling EEE
> > advertisement on the PHY prevent this feature to be negotiated.
> > With this change, the link is stable and reliable, with the
> > expected
> > throughput performance.
> > 
> > The patchset adds options in the generic phy driver to disable EEE
> > advertisement, through device tree. The way it is done is very
> > similar
> > to the handling of the max-speed property.
> > 
> > Patch 4 is provided here for testing purpose only. Please don't
> > merge
> > patch 4, this change will go through the amlogic's tree.
> 
> Sorry, but I really don't like the route this is going, and I should
> have made myself clearer before on that, I really think utilizing a
> PHY
> fixup is more appropriate here than an extremely generic DT property.
> The fixup code can be in the affected PHY driver, or it can be
> somewhere
> else, your call. There is no shortage of option on how to implement
> it,
> and this would be something easy to enable/disable for known good
> configurations (ala PCI/USB fixups).
> 
> If we start supporting generic "enable", "disable" type of properties
> with values that map directly to register definitions of the HW, we
> leave too much room for these properties to be utilized to implement
> a
> specific policy, and this is not acceptable.

Florian,?

I agree that DT should not be used to setup a policy, but to describe
what the HW is.

I tried to implement it the way you suggested, using phy fixup, too see
what it looks like.
There is 2 places in the code that seems (remotely) linked to the
issue:?
- meson8b_dwmac driver : if the mac, regardless of the board/platform,
?could not tolerate to have EEE activated, it would make sense to have
the fixup here. It can provide a C callback for such case.
- realtek phy driver: philosophy is kind of the same

To be clear, it is doable and it works that way, but I don't think
embedding this directly in the code is the right way to do it. It seems
we are hiding an information specific about the board inside a generic
driver.

We have several amlogic's design with the same MAC, sometimes with the
same PHY, which have no problem with EEE at all. The issue is really
about the board design.

What I propose is not an enable/disable configuration switch, but to
clearly state that a particular mode of operation is broken. Like the
"max-speed" property, it setup a restriction. IMO, this is a
description of what the HW is and is capable of, and as such it should
be part of the DT.

Yes the property directly map to a register, but it does let you
directly manipulate it (you can't pass the value you want to write in
the register). Having it this way just makes the code simple on both
ends (user and driver).

Yes people could start abusing this to setup policy. In the end, it is
our responsibility, as community, to make sure APIs are used in a
proper way, and not let it be used that way.

I'm open to suggestion on how improve the solution, maybe something
which could bring more confidence that property won't be misused.

Jerome

^ permalink raw reply

* [PATCH v2 5/5] arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161130094351.2748-1-romain.perier@free-electrons.com>

This commit enables the device node spi0 on the official development
board for the Marvell Armada 3700. It also adds sub-node for the 128Mb
SPI-NOR present on the board.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 ++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 1372e9a6..0c4eb98 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -67,6 +67,36 @@
 	status = "okay";
 };
 
+&spi0 {
+	status = "okay";
+
+	m25p80 at 0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <108000000>;
+		spi-rx-bus-width = <4>;
+		spi-tx-bus-width = <4>;
+
+		partitions {
+			compatible = "fixed-partitions";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			partition at 0 {
+				label = "bootloader";
+				reg = <0x0 0x200000>;
+			};
+			partition at 200000 {
+				label = "U-boot Env";
+				reg = <0x200000 0x10000>;
+			};
+			partition at 210000 {
+				label = "Linux";
+				reg = <0x210000 0xDF0000>;
+			};
+		};
+	};
+};
+
 /* Exported on the micro USB connector CON32 through an FTDI */
 &uart0 {
 	status = "okay";
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 4/5] arm64: dts: marvell: Add definition of SPI controller for Armada 3700
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161130094351.2748-1-romain.perier@free-electrons.com>

Armada 3700 SoC has an SPI Controller, this commit adds the definition
of the SPI device node at the SoC level.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---

Changes in v2:
 - Removed properties max-frequency and clock-frequency, it is no
   longer required and not used by the DT-bindings.

 arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index e9bd587..63c2002 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -98,6 +98,17 @@
 			/* 32M internal register @ 0xd000_0000 */
 			ranges = <0x0 0x0 0xd0000000 0x2000000>;
 
+			spi0: spi at 10600 {
+				compatible = "marvell,armada-3700-spi";
+				#address-cells = <1>;
+				#size-cells = <0>;
+				reg = <0x10600 0x5d>;
+				clocks = <&nb_periph_clk 7>;
+				interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+				num-cs = <4>;
+				status = "disabled";
+			};
+
 			uart0: serial at 12000 {
 				compatible = "marvell,armada-3700-uart";
 				reg = <0x12000 0x400>;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 3/5] dt-bindings: spi: Add documentation for the Armada 3700 SPI Controller
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161130094351.2748-1-romain.perier@free-electrons.com>

This adds the devicetree bindings documentation for the SPI controller
present in the Marvell Armada 3700 SoCs.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 .../devicetree/bindings/spi/spi-armada-3700.txt    | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt

diff --git a/Documentation/devicetree/bindings/spi/spi-armada-3700.txt b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
new file mode 100644
index 0000000..1564aa8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-armada-3700.txt
@@ -0,0 +1,25 @@
+* Marvell Armada 3700 SPI Controller
+
+Required Properties:
+
+- compatible: should be "marvell,armada-3700-spi"
+- reg: physical base address of the controller and length of memory mapped
+       region.
+- interrupts: The interrupt number. The interrupt specifier format depends on
+	      the interrupt controller and of its driver.
+- clocks: Must contain the clock source, usually from the North Bridge clocks.
+- num-cs: The number of chip selects that is supported by this SPI Controller
+- #address-cells: should be 1.
+- #size-cells: should be 0.
+
+Example:
+
+	spi0: spi at 10600 {
+		compatible = "marvell,armada-3700-spi";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reg = <0x10600 0x5d>;
+		clocks = <&nb_perih_clk 7>;
+		interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+		num-cs = <4>;
+	};
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 2/5] spi: armada-3700: Add support for the FIFO mode
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161130094351.2748-1-romain.perier@free-electrons.com>

In FIFO mode, dedicated registers are used to store the instruction,
the address, the read mode and the data. Write and Read FIFO are used
to store the outcoming or incoming data. The CPU no longer has to assert
each byte. The data FIFOs are accessible via DMA or by the CPU.

This commit adds support for the FIFO mode with the CPU.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---

Changes in v2:
 - Removed a3700_spi_bytelen_set from the setup function, it was accidentally
   let here and not required, as it is configured in the prepare callback now
   (defaults to 4 for fifo mode). It solves unrecognized spi-nor flash memory
   detection with jedec.

 drivers/spi/spi-armada-3700.c | 409 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 399 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index cc9e1b2..72f1818 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -99,19 +99,28 @@
 /* A3700_SPI_IF_TIME_REG */
 #define A3700_SPI_CLK_CAPT_EDGE		BIT(7)
 
+/* Flags and macros for struct a3700_spi */
+#define HAS_FIFO			BIT(0)
+#define A3700_INSTR_CNT			1
+#define A3700_ADDR_CNT			3
+#define A3700_DUMMY_CNT			1
+
 struct a3700_spi {
 	struct spi_master *master;
 	void __iomem *base;
 	struct clk *clk;
 	unsigned int irq;
 	unsigned int flags;
-	bool last_xfer;
+	bool xmit_data;
 	const u8 *tx_buf;
 	u8 *rx_buf;
 	size_t buf_len;
 	u8 byte_len;
 	u32 wait_mask;
 	struct completion done;
+	u32 addr_cnt;
+	u32 instr_cnt;
+	size_t hdr_cnt;
 };
 
 static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
@@ -180,12 +189,15 @@ static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
 	return 0;
 }
 
-static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+static void a3700_spi_fifo_mode_set(struct a3700_spi *a3700_spi)
 {
 	u32 val;
 
 	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
-	val &= ~A3700_SPI_FIFO_MODE;
+	if (a3700_spi->flags & HAS_FIFO)
+		val |= A3700_SPI_FIFO_MODE;
+	else
+		val &= ~A3700_SPI_FIFO_MODE;
 	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
 }
 
@@ -255,11 +267,30 @@ static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
 	a3700_spi->byte_len = len;
 }
 
+static int a3700_spi_fifo_flush(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_FIFO_FLUSH;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_FIFO_FLUSH))
+			return 0;
+		udelay(1);
+	}
+
+	return -ETIMEDOUT;
+}
+
 static int a3700_spi_init(struct a3700_spi *a3700_spi)
 {
 	struct spi_master *master = a3700_spi->master;
 	u32 val;
-	int i;
+	int i, ret = 0;
 
 	/* Reset SPI unit */
 	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
@@ -278,10 +309,8 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
 	for (i = 0; i < master->num_chipselect; i++)
 		a3700_spi_deactivate_cs(a3700_spi, i);
 
-	a3700_spi_pin_mode_set(a3700_spi, 0);
-
-	/* Be sure that FIFO mode is disabled */
-	a3700_spi_fifo_mode_unset(a3700_spi);
+	/* Enable FIFO mode */
+	a3700_spi_fifo_mode_set(a3700_spi);
 
 	/* Set SPI mode */
 	a3700_spi_mode_set(a3700_spi, master->mode_bits);
@@ -294,7 +323,7 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
 	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
 	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
 
-	return 0;
+	return ret;
 }
 
 static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
@@ -380,14 +409,34 @@ static bool a3700_spi_transfer_wait(struct spi_device *spi,
 	return a3700_spi_wait_completion(spi);
 }
 
+static void a3700_spi_fifo_thres_set(struct a3700_spi *a3700_spi,
+				     unsigned int bytes)
+{
+	u32 val;
+
+	if (a3700_spi->flags & HAS_FIFO) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_RFIFO_THRS_BIT);
+		val |= (bytes - 1) << A3700_SPI_RFIFO_THRS_BIT;
+		val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_WFIFO_THRS_BIT);
+		val |= (7 - bytes) << A3700_SPI_WFIFO_THRS_BIT;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	}
+}
+
 static void a3700_spi_transfer_setup(struct spi_device *spi,
 				    struct spi_transfer *xfer)
 {
 	struct a3700_spi *a3700_spi;
+	unsigned int byte_len;
 
 	a3700_spi = spi_master_get_devdata(spi->master);
 
 	a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+
+	byte_len = xfer->bits_per_word >> 3;
+
+	a3700_spi_fifo_thres_set(a3700_spi, byte_len);
 }
 
 static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
@@ -447,6 +496,168 @@ static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
 		a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
 }
 
+static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
+{
+	u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+	u32 val = 0;
+
+	/* Clear the header registers */
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+
+	/* Set header counters */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
+			instr_cnt = a3700_spi->buf_len;
+		} else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
+						  a3700_spi->addr_cnt)) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->buf_len - instr_cnt;
+		} else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
+			instr_cnt = a3700_spi->instr_cnt;
+			addr_cnt = a3700_spi->addr_cnt;
+			/* Need to handle the normal write case with 1 byte
+			 * data
+			 */
+			if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
+				dummy_cnt = a3700_spi->buf_len - instr_cnt -
+					    addr_cnt;
+		}
+		val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
+			<< A3700_SPI_INSTR_CNT_BIT);
+		val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+			<< A3700_SPI_ADDR_CNT_BIT);
+		val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
+			<< A3700_SPI_DUMMY_CNT_BIT);
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+	/* Update the buffer length to be transferred */
+	a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
+
+	/* Set Instruction */
+	val = 0;
+	while (instr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
+
+	/* Set Address */
+	val = 0;
+	while (addr_cnt--) {
+		val = (val << 8) | a3700_spi->tx_buf[0];
+		a3700_spi->tx_buf++;
+	}
+	spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
+}
+
+static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	return (val & A3700_SPI_WFIFO_FULL);
+}
+
+static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+	int i = 0;
+
+	while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
+		val = 0;
+		if (a3700_spi->buf_len >= 4) {
+			val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+			spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->tx_buf += 4;
+		} else {
+			/*
+			 * If the remained buffer length is less than 4-bytes,
+			 * we should pad the write buffer with all ones. So that
+			 * it avoids overwrite the unexpected bytes following
+			 * the last one.
+			 */
+			val = GENMASK(31, 0);
+			while (a3700_spi->buf_len) {
+				val &= ~(0xff << (8 * i));
+				val |= *a3700_spi->tx_buf++ << (8 * i);
+				i++;
+				a3700_spi->buf_len--;
+
+				spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
+					     val);
+			}
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static int a3700_is_rfifo_empty(struct a3700_spi *a3700_spi)
+{
+	u32 val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+
+	return (val & A3700_SPI_RFIFO_EMPTY);
+}
+
+static int a3700_spi_fifo_read(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	while (!a3700_is_rfifo_empty(a3700_spi) && a3700_spi->buf_len) {
+		val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+		if (a3700_spi->buf_len >= 4) {
+			u32 data = le32_to_cpu(val);
+			memcpy(a3700_spi->rx_buf, &data, 4);
+
+			a3700_spi->buf_len -= 4;
+			a3700_spi->rx_buf += 4;
+		} else {
+			/*
+			 * When remain bytes is not larger than 4, we should
+			 * avoid memory overwriting and just write the left rx
+			 * buffer bytes.
+			 */
+			while (a3700_spi->buf_len) {
+				*a3700_spi->rx_buf = val & 0xff;
+				val >>= 8;
+
+				a3700_spi->buf_len--;
+				a3700_spi->rx_buf++;
+			}
+		}
+	}
+
+	return 0;
+}
+
+static void a3700_spi_transfer_abort_fifo(struct a3700_spi *a3700_spi)
+{
+	int timeout = A3700_SPI_TIMEOUT;
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	a3700_spi_fifo_flush(a3700_spi);
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
 static int a3700_spi_prepare_message(struct spi_master *master,
 				     struct spi_message *message)
 {
@@ -463,12 +674,28 @@ static int a3700_spi_prepare_message(struct spi_master *master,
 	return 0;
 }
 
+static int a3700_spi_prepare_fifo_message(struct spi_master *master,
+					  struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret;
+
+	/* Flush the FIFOs */
+	ret = a3700_spi_fifo_flush(a3700_spi);
+	if (ret)
+		return ret;
+
+	a3700_spi_bytelen_set(a3700_spi, 4);
+
+	return 0;
+}
+
 static int a3700_spi_transfer_one(struct spi_master *master,
 				  struct spi_device *spi,
 				  struct spi_transfer *xfer)
 {
 	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
-	int ret = 0;
+	int ret;
 
 	a3700_spi_transfer_setup(spi, xfer);
 
@@ -505,6 +732,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
 	return ret;
 }
 
+static int a3700_spi_fifo_transfer_one(struct spi_master *master,
+				       struct spi_device *spi,
+				       struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret = 0, timeout = A3700_SPI_TIMEOUT;
+	unsigned int nbits = 0;
+	u32 val;
+
+	a3700_spi_transfer_setup(spi, xfer);
+
+	a3700_spi->tx_buf  = xfer->tx_buf;
+	a3700_spi->rx_buf  = xfer->rx_buf;
+	a3700_spi->buf_len = xfer->len;
+
+	/* SPI transfer headers */
+	a3700_spi_header_set(a3700_spi);
+
+	if (xfer->tx_buf)
+		nbits = xfer->tx_nbits;
+	else if (xfer->rx_buf)
+		nbits = xfer->rx_nbits;
+
+	a3700_spi_pin_mode_set(a3700_spi, nbits);
+
+	if (xfer->rx_buf) {
+		/* Set read data length */
+		spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
+			     a3700_spi->buf_len);
+		/* Start READ transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val &= ~A3700_SPI_RW_EN;
+		val |= A3700_SPI_XFER_START;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	} else if (xfer->tx_buf) {
+		/* Start Write transfer */
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= (A3700_SPI_XFER_START | A3700_SPI_RW_EN);
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+		/*
+		 * If there are data to be written to the SPI device, xmit_data
+		 * flag is set true; otherwise the instruction in SPI_INSTR does
+		 * not require data to be written to the SPI device, then
+		 * xmit_data flag is set false.
+		 */
+		a3700_spi->xmit_data = (a3700_spi->buf_len != 0);
+	}
+
+	while (a3700_spi->buf_len) {
+		if (a3700_spi->tx_buf) {
+			/* Wait wfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait wfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Fill up the wfifo */
+			ret = a3700_spi_fifo_write(a3700_spi);
+			if (ret)
+				goto error;
+		} else if (a3700_spi->rx_buf) {
+			/* Wait rfifo ready */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_RFIFO_RDY)) {
+				dev_err(&spi->dev,
+					"wait rfifo ready timed out\n");
+				ret = -ETIMEDOUT;
+				goto error;
+			}
+			/* Drain out the rfifo */
+			ret = a3700_spi_fifo_read(a3700_spi);
+			if (ret)
+				goto error;
+		}
+	}
+
+	/*
+	 * Stop a write transfer in fifo mode:
+	 *	- wait all the bytes in wfifo to be shifted out
+	 *	 - set XFER_STOP bit
+	 *	- wait XFER_START bit clear
+	 *	- clear XFER_STOP bit
+	 * Stop a read transfer in fifo mode:
+	 *	- the hardware is to reset the XFER_START bit
+	 *	   after the number of bytes indicated in DIN_CNT
+	 *	   register
+	 *	- just wait XFER_START bit clear
+	 */
+	if (a3700_spi->tx_buf) {
+		if (a3700_spi->xmit_data) {
+			/*
+			 * If there are data written to the SPI device, wait
+			 * until SPI_WFIFO_EMPTY is 1 to wait for all data to
+			 * transfer out of write FIFO.
+			 */
+			if (!a3700_spi_transfer_wait(spi,
+						     A3700_SPI_WFIFO_EMPTY)) {
+				dev_err(&spi->dev, "wait wfifo empty timed out\n");
+				return -ETIMEDOUT;
+			}
+		} else {
+			/*
+			 * If the instruction in SPI_INSTR does not require data
+			 * to be written to the SPI device, wait until SPI_RDY
+			 * is 1 for the SPI interface to be in idle.
+			 */
+			if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+				dev_err(&spi->dev, "wait xfer ready timed out\n");
+				return -ETIMEDOUT;
+			}
+		}
+
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		val |= A3700_SPI_XFER_STOP;
+		spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	}
+
+	while (--timeout) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+		if (!(val & A3700_SPI_XFER_START))
+			break;
+		udelay(1);
+	}
+
+	if (timeout == 0) {
+		dev_err(&spi->dev, "wait transfer start clear timed out\n");
+		ret = -ETIMEDOUT;
+		goto error;
+	}
+
+	val &= ~A3700_SPI_XFER_STOP;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+	goto out;
+
+error:
+	a3700_spi_transfer_abort_fifo(a3700_spi);
+out:
+	spi_finalize_current_transfer(master);
+
+	return ret;
+}
+
 static int a3700_spi_unprepare_message(struct spi_master *master,
 				       struct spi_message *message)
 {
@@ -592,6 +964,23 @@ static int a3700_spi_probe(struct platform_device *pdev)
 		goto error;
 	}
 
+	if (of_device_is_compatible(of_node, "marvell,armada-3700-spi")) {
+		master->prepare_message =  a3700_spi_prepare_fifo_message;
+		master->transfer_one = a3700_spi_fifo_transfer_one;
+
+		spi->flags |= HAS_FIFO;
+		spi->instr_cnt = A3700_INSTR_CNT;
+		spi->addr_cnt = A3700_ADDR_CNT;
+		spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
+			       A3700_DUMMY_CNT;
+		master->mode_bits |= (SPI_RX_DUAL | SPI_RX_DUAL |
+				      SPI_RX_QUAD | SPI_TX_QUAD);
+	} else {
+		master->prepare_message =  a3700_spi_prepare_message;
+		master->transfer_one = a3700_spi_transfer_one;
+		master->unprepare_message = a3700_spi_unprepare_message;
+	}
+
 	ret = a3700_spi_init(spi);
 	if (ret)
 		goto error_clk;
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161130094351.2748-1-romain.perier@free-electrons.com>

Marvell Armada 3700 SoC comprises an SPI Controller. This Controller
supports up to 4 SPI slave devices, with dedicated chip selects, supports
SPI mode 0/1/2 and 3, CPIO or Fifo mode with DMA transfers and different
SPI transfer mode (Single, Dual or Quad).

This commit adds basic driver support for CPIO mode and single SPI
transfer. In this mode, the CPU asserts cs, outputs or inputs data from
the current SPI device. Data transfers are copied by 1 or 4 bytes using
the SPI registers.

Signed-off-by: Romain Perier <romain.perier@free-electrons.com>
---
 drivers/spi/Kconfig           |   7 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-armada-3700.c | 651 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 659 insertions(+)
 create mode 100644 drivers/spi/spi-armada-3700.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..6ade1ca 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -67,6 +67,13 @@ config SPI_ATH79
 	  This enables support for the SPI controller present on the
 	  Atheros AR71XX/AR724X/AR913X SoCs.
 
+config SPI_ARMADA_3700
+	tristate "Marvell Armada 3700 SPI Controller"
+	depends on ARCH_MVEBU && OF
+	help
+	  This enables support for the SPI controller present on the
+	  Marvell Armada 3700 SoCs.
+
 config SPI_ATMEL
 	tristate "Atmel SPI Controller"
 	depends on HAS_DMA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index aa939d9..140ca45 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 
 # SPI master controller drivers (bus)
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
+obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
new file mode 100644
index 0000000..cc9e1b2
--- /dev/null
+++ b/drivers/spi/spi-armada-3700.c
@@ -0,0 +1,651 @@
+/*
+ * Marvell Armada-3700 SPI controller driver
+ *
+ * Copyright (C) 2016 Marvell Ltd.
+ *
+ * Author: Wilson Ding <dingwei@marvell.com>
+ * Author: Romain Perier <romain.perier@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME			"armada_3700_spi"
+
+#define A3700_SPI_TIMEOUT		10
+
+/* SPI Register Offest */
+#define A3700_SPI_IF_CTRL_REG		0x00
+#define A3700_SPI_IF_CFG_REG		0x04
+#define A3700_SPI_DATA_OUT_REG		0x08
+#define A3700_SPI_DATA_IN_REG		0x0C
+#define A3700_SPI_IF_INST_REG		0x10
+#define A3700_SPI_IF_ADDR_REG		0x14
+#define A3700_SPI_IF_RMODE_REG		0x18
+#define A3700_SPI_IF_HDR_CNT_REG	0x1C
+#define A3700_SPI_IF_DIN_CNT_REG	0x20
+#define A3700_SPI_IF_TIME_REG		0x24
+#define A3700_SPI_INT_STAT_REG		0x28
+#define A3700_SPI_INT_MASK_REG		0x2C
+
+/* A3700_SPI_IF_CTRL_REG */
+#define A3700_SPI_EN			BIT(16)
+#define A3700_SPI_ADDR_NOT_CONFIG	BIT(12)
+#define A3700_SPI_WFIFO_OVERFLOW	BIT(11)
+#define A3700_SPI_WFIFO_UNDERFLOW	BIT(10)
+#define A3700_SPI_RFIFO_OVERFLOW	BIT(9)
+#define A3700_SPI_RFIFO_UNDERFLOW	BIT(8)
+#define A3700_SPI_WFIFO_FULL		BIT(7)
+#define A3700_SPI_WFIFO_EMPTY		BIT(6)
+#define A3700_SPI_RFIFO_FULL		BIT(5)
+#define A3700_SPI_RFIFO_EMPTY		BIT(4)
+#define A3700_SPI_WFIFO_RDY		BIT(3)
+#define A3700_SPI_RFIFO_RDY		BIT(2)
+#define A3700_SPI_XFER_RDY		BIT(1)
+#define A3700_SPI_XFER_DONE		BIT(0)
+
+/* A3700_SPI_IF_CFG_REG */
+#define A3700_SPI_WFIFO_THRS		BIT(28)
+#define A3700_SPI_RFIFO_THRS		BIT(24)
+#define A3700_SPI_AUTO_CS		BIT(20)
+#define A3700_SPI_DMA_RD_EN		BIT(18)
+#define A3700_SPI_FIFO_MODE		BIT(17)
+#define A3700_SPI_SRST			BIT(16)
+#define A3700_SPI_XFER_START		BIT(15)
+#define A3700_SPI_XFER_STOP		BIT(14)
+#define A3700_SPI_INST_PIN		BIT(13)
+#define A3700_SPI_ADDR_PIN		BIT(12)
+#define A3700_SPI_DATA_PIN1		BIT(11)
+#define A3700_SPI_DATA_PIN0		BIT(10)
+#define A3700_SPI_FIFO_FLUSH		BIT(9)
+#define A3700_SPI_RW_EN			BIT(8)
+#define A3700_SPI_CLK_POL		BIT(7)
+#define A3700_SPI_CLK_PHA		BIT(6)
+#define A3700_SPI_BYTE_LEN		BIT(5)
+#define A3700_SPI_CLK_PRESCALE		BIT(0)
+#define A3700_SPI_CLK_PRESCALE_MASK	(0x1f)
+
+#define A3700_SPI_WFIFO_THRS_BIT	28
+#define A3700_SPI_RFIFO_THRS_BIT	24
+#define A3700_SPI_FIFO_THRS_MASK	0x7
+
+#define A3700_SPI_DATA_PIN_MASK		0x3
+
+/* A3700_SPI_IF_HDR_CNT_REG */
+#define A3700_SPI_DUMMY_CNT_BIT		12
+#define A3700_SPI_DUMMY_CNT_MASK	0x7
+#define A3700_SPI_RMODE_CNT_BIT		8
+#define A3700_SPI_RMODE_CNT_MASK	0x3
+#define A3700_SPI_ADDR_CNT_BIT		4
+#define A3700_SPI_ADDR_CNT_MASK		0x7
+#define A3700_SPI_INSTR_CNT_BIT		0
+#define A3700_SPI_INSTR_CNT_MASK	0x3
+
+/* A3700_SPI_IF_TIME_REG */
+#define A3700_SPI_CLK_CAPT_EDGE		BIT(7)
+
+struct a3700_spi {
+	struct spi_master *master;
+	void __iomem *base;
+	struct clk *clk;
+	unsigned int irq;
+	unsigned int flags;
+	bool last_xfer;
+	const u8 *tx_buf;
+	u8 *rx_buf;
+	size_t buf_len;
+	u8 byte_len;
+	u32 wait_mask;
+	struct completion done;
+};
+
+static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
+{
+	return readl(a3700_spi->base + offset);
+}
+
+static void spireg_write(struct a3700_spi *a3700_spi, u32 offset, u32 data)
+{
+	writel(data, a3700_spi->base + offset);
+}
+
+static void a3700_spi_auto_cs_unset(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_AUTO_CS;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_activate_cs(struct a3700_spi *a3700_spi, unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val |= (A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static void a3700_spi_deactivate_cs(struct a3700_spi *a3700_spi,
+				    unsigned int cs)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	val &= ~(A3700_SPI_EN << cs);
+	spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
+				  unsigned int pin_mode)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_INST_PIN | A3700_SPI_ADDR_PIN);
+	val &= ~(A3700_SPI_DATA_PIN0 | A3700_SPI_DATA_PIN1);
+
+	switch (pin_mode) {
+	case 1:
+		break;
+	case 2:
+		val |= A3700_SPI_DATA_PIN0;
+		break;
+	case 4:
+		val |= A3700_SPI_DATA_PIN1;
+		break;
+	default:
+		dev_err(&a3700_spi->master->dev, "wrong pin mode %u", pin_mode);
+		return -EINVAL;
+	}
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	return 0;
+}
+
+static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_FIFO_MODE;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_mode_set(struct a3700_spi *a3700_spi,
+			       unsigned int mode_bits)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+
+	if (mode_bits & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+	else
+		val &= ~A3700_SPI_CLK_POL;
+
+	if (mode_bits & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+	else
+		val &= ~A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_clock_set(struct a3700_spi *a3700_spi,
+				unsigned int speed_hz, u16 mode)
+{
+	u32 val;
+	u32 prescale;
+
+	prescale = DIV_ROUND_UP(clk_get_rate(a3700_spi->clk), speed_hz);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val = val & ~A3700_SPI_CLK_PRESCALE_MASK;
+
+	val = val | (prescale & A3700_SPI_CLK_PRESCALE_MASK);
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	if (prescale <= 2) {
+		val = spireg_read(a3700_spi, A3700_SPI_IF_TIME_REG);
+		val |= A3700_SPI_CLK_CAPT_EDGE;
+		spireg_write(a3700_spi, A3700_SPI_IF_TIME_REG, val);
+	}
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~(A3700_SPI_CLK_POL | A3700_SPI_CLK_PHA);
+
+	if (mode & SPI_CPOL)
+		val |= A3700_SPI_CLK_POL;
+
+	if (mode & SPI_CPHA)
+		val |= A3700_SPI_CLK_PHA;
+
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
+{
+	u32 val;
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	if (len == 4)
+		val |= A3700_SPI_BYTE_LEN;
+	else
+		val &= ~A3700_SPI_BYTE_LEN;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	a3700_spi->byte_len = len;
+}
+
+static int a3700_spi_init(struct a3700_spi *a3700_spi)
+{
+	struct spi_master *master = a3700_spi->master;
+	u32 val;
+	int i;
+
+	/* Reset SPI unit */
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val |= A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	for (i = 0; i < A3700_SPI_TIMEOUT; i++)
+		udelay(1);
+
+	val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+	val &= ~A3700_SPI_SRST;
+	spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+	/* Disable AUTO_CS and deactivate all chip-selects */
+	a3700_spi_auto_cs_unset(a3700_spi);
+	for (i = 0; i < master->num_chipselect; i++)
+		a3700_spi_deactivate_cs(a3700_spi, i);
+
+	a3700_spi_pin_mode_set(a3700_spi, 0);
+
+	/* Be sure that FIFO mode is disabled */
+	a3700_spi_fifo_mode_unset(a3700_spi);
+
+	/* Set SPI mode */
+	a3700_spi_mode_set(a3700_spi, master->mode_bits);
+
+	/* Reset counters */
+	spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG, 0);
+
+	/* Mask the interrupts and clear cause bits */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
+
+	return 0;
+}
+
+static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_master *master = dev_id;
+	struct a3700_spi *a3700_spi;
+	u32 cause;
+
+	a3700_spi = spi_master_get_devdata(master);
+
+	/* Get interrupt causes */
+	cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
+
+	/* mask and acknowledge the SPI interrupts */
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+	spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
+
+	/* Wake up the transfer */
+	if (a3700_spi->wait_mask & cause)
+		complete(&a3700_spi->done);
+
+	return IRQ_HANDLED;
+}
+
+static bool a3700_spi_wait_completion(struct spi_device *spi)
+{
+	struct a3700_spi *a3700_spi;
+	unsigned int timeout;
+	unsigned int ctrl_reg;
+	unsigned long timeout_jiffies;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	/* SPI interrupt is edge-triggered, which means an interrupt will
+	 * be generated only when detecting a specific status bit changed
+	 * from '0' to '1'. So when we start waiting for a interrupt, we
+	 * need to check status bit in control reg first, if it is already 1,
+	 * then we do not need to wait for interrupt
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	reinit_completion(&a3700_spi->done);
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG,
+		     a3700_spi->wait_mask);
+
+	timeout_jiffies = msecs_to_jiffies(A3700_SPI_TIMEOUT);
+	timeout = wait_for_completion_timeout(&a3700_spi->done,
+					      timeout_jiffies);
+
+	a3700_spi->wait_mask = 0;
+
+	if (timeout)
+		return true;
+
+	/* there might be the case that right after we checked the
+	 * status bits in this routine and before start to wait for
+	 * interrupt by wait_for_completion_timeout, the interrupt
+	 * happens, to avoid missing it we need to double check
+	 * status bits in control reg, if it is already 1, then
+	 * consider that we have the interrupt successfully and
+	 * return true.
+	 */
+	ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+	if (a3700_spi->wait_mask & ctrl_reg)
+		return true;
+
+	spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+
+	return true;
+}
+
+static bool a3700_spi_transfer_wait(struct spi_device *spi,
+				    unsigned int bit_mask)
+{
+	struct a3700_spi *a3700_spi;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+	a3700_spi->wait_mask = bit_mask;
+
+	return a3700_spi_wait_completion(spi);
+}
+
+static void a3700_spi_transfer_setup(struct spi_device *spi,
+				    struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi;
+
+	a3700_spi = spi_master_get_devdata(spi->master);
+
+	a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+}
+
+static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
+{
+	u32 val, data;
+
+	if (a3700_spi->buf_len % a3700_spi->byte_len)
+		return -EINVAL;
+
+	/* Read bytes from data in register */
+	val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+
+	if (a3700_spi->byte_len == 4)
+		data = be32_to_cpu(val);
+	else
+		data = val;
+
+	memcpy(a3700_spi->rx_buf, &data, a3700_spi->byte_len);
+
+	a3700_spi->buf_len -= a3700_spi->byte_len;
+	a3700_spi->rx_buf  += a3700_spi->byte_len;
+
+	/* Request next 1 or 4 bytes data */
+	if (a3700_spi->buf_len)
+		spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+	return 0;
+}
+
+static int a3700_spi_write_data(struct a3700_spi *a3700_spi)
+{
+	u32 val = 0;
+
+	if (a3700_spi->buf_len % a3700_spi->byte_len)
+		return -EINVAL;
+
+	/* Write bytes from data out register */
+	if (a3700_spi->byte_len == 4)
+		val = cpu_to_be32(*(u32 *)a3700_spi->tx_buf);
+	else
+		val = a3700_spi->tx_buf[0];
+
+	spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+	a3700_spi->buf_len -= a3700_spi->byte_len;
+	a3700_spi->tx_buf  += a3700_spi->byte_len;
+
+	return 0;
+}
+
+static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(spi->master);
+
+	if (!enable)
+		a3700_spi_activate_cs(a3700_spi, spi->chip_select);
+	else
+		a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
+}
+
+static int a3700_spi_prepare_message(struct spi_master *master,
+				     struct spi_message *message)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	struct spi_device *spi = message->spi;
+
+	a3700_spi_bytelen_set(a3700_spi, 1);
+
+	if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+		dev_err(&spi->dev, "wait transfer ready timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int a3700_spi_transfer_one(struct spi_master *master,
+				  struct spi_device *spi,
+				  struct spi_transfer *xfer)
+{
+	struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+	int ret = 0;
+
+	a3700_spi_transfer_setup(spi, xfer);
+
+	a3700_spi->tx_buf  = xfer->tx_buf;
+	a3700_spi->rx_buf  = xfer->rx_buf;
+	a3700_spi->buf_len = xfer->len;
+
+	/* Start READ transfer by writing dummy data to DOUT register */
+	if (xfer->rx_buf)
+		spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+	while (a3700_spi->buf_len) {
+		if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+			dev_err(&spi->dev, "wait transfer ready timed out\n");
+			ret = -ETIMEDOUT;
+			goto err;
+		}
+
+		if (a3700_spi->tx_buf) {
+			ret = a3700_spi_write_data(a3700_spi);
+			if (ret)
+				goto err;
+		}
+
+		if (a3700_spi->rx_buf) {
+			ret = a3700_spi_read_data(a3700_spi);
+			if (ret)
+				goto err;
+		}
+	}
+
+err:
+	spi_finalize_current_transfer(master);
+	return ret;
+}
+
+static int a3700_spi_unprepare_message(struct spi_master *master,
+				       struct spi_message *message)
+{
+	struct spi_device *spi = message->spi;
+
+	if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+		dev_err(&spi->dev, "wait transfer ready timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id a3700_spi_dt_ids[] = {
+	{ .compatible = "marvell,armada-3700-spi", .data = NULL },
+};
+
+MODULE_DEVICE_TABLE(of, a3700_spi_of_match_table);
+
+static int a3700_spi_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *of_node = dev->of_node;
+	struct resource *res;
+	struct spi_master *master;
+	struct a3700_spi *spi;
+	u32 num_cs = 0;
+	int ret = 0;
+
+	master = spi_alloc_master(dev, sizeof(*spi));
+	if (!master) {
+		dev_err(dev, "master allocation failed\n");
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	if (of_property_read_u32(of_node, "num-cs", &num_cs)) {
+		dev_err(dev, "could not find num-cs\n");
+		ret = -ENXIO;
+		goto error;
+	}
+
+	master->bus_num = (pdev->id != -1) ? pdev->id : 0;
+	master->dev.of_node = of_node;
+	master->mode_bits = SPI_MODE_3;
+	master->num_chipselect = num_cs;
+	master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(32);
+	master->prepare_message =  a3700_spi_prepare_message;
+	master->transfer_one = a3700_spi_transfer_one;
+	master->unprepare_message = a3700_spi_unprepare_message;
+	master->set_cs = a3700_spi_set_cs;
+
+	platform_set_drvdata(pdev, master);
+
+	spi = spi_master_get_devdata(master);
+	memset(spi, 0, sizeof(struct a3700_spi));
+
+	spi->master = master;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	spi->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(spi->base)) {
+		ret = PTR_ERR(spi->base);
+		goto error;
+	}
+
+	spi->irq = platform_get_irq(pdev, 0);
+	if (spi->irq < 0) {
+		dev_err(dev, "could not get irq: %d\n", spi->irq);
+		ret = -ENXIO;
+		goto error;
+	}
+
+	init_completion(&spi->done);
+
+	spi->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(spi->clk)) {
+		dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk));
+		goto error;
+	}
+
+	ret = clk_prepare_enable(spi->clk);
+	if (ret) {
+		dev_err(dev, "could not prepare clk: %d\n", ret);
+		goto error;
+	}
+
+	ret = a3700_spi_init(spi);
+	if (ret)
+		goto error_clk;
+
+	ret = devm_request_irq(dev, spi->irq, a3700_spi_interrupt, 0,
+			       dev_name(dev), master);
+	if (ret) {
+		dev_err(dev, "could not request IRQ: %d\n", ret);
+		goto error_clk;
+	}
+
+	ret = devm_spi_register_master(dev, master);
+	if (ret) {
+		dev_err(dev, "Failed to register master\n");
+		goto error_clk;
+	}
+
+	dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
+		 (unsigned long)res->start, spi->irq);
+
+	return 0;
+
+error_clk:
+	clk_disable_unprepare(spi->clk);
+error:
+	spi_master_put(master);
+out:
+	return ret;
+}
+
+static int a3700_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct a3700_spi *spi = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(spi->clk);
+	spi_master_put(master);
+
+	return 0;
+}
+
+static struct platform_driver a3700_spi_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(a3700_spi_dt_ids),
+	},
+	.probe		= a3700_spi_probe,
+	.remove		= a3700_spi_remove,
+};
+
+module_platform_driver(a3700_spi_driver);
+
+MODULE_DESCRIPTION("Armada-3700 SPI driver");
+MODULE_AUTHOR("Wilson Ding <dingwei@marvell.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.9.3

^ permalink raw reply related

* [PATCH v2 0/5] Add support for the Armada 3700 SPI controller
From: Romain Perier @ 2016-11-30  9:43 UTC (permalink / raw)
  To: linux-arm-kernel

The Marvell Armada 3700 SoC includes an SPI controller. This controller
supports up to 4 SPI slave devices, with dedicated chip selects, CPIO or
FIFO mode with DMA or CPU transfers and different SPI transfer modes
(Standard single, Dual or Quad).

This set of patches adds a basic support for the CPIO mode, then it
enables the FIFO mode (CPU-side only, DMA not supported yet). It also
adds the required definitions of the spi nodes to the devicetree.

Romain Perier (5):
  spi: Add basic support for Armada 3700 SPI Controller
  spi: armada-3700: Add support for the FIFO mode
  dt-bindings: spi: Add documentation for the Armada 3700 SPI Controller
  arm64: dts: marvell: Add definition of SPI controller for Armada 3700
  arm64: dts: marvell: Enable spi0 on the board Armada-3720-db

 .../devicetree/bindings/spi/spi-armada-3700.txt    |   25 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |   30 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   13 +
 drivers/spi/Kconfig                                |    7 +
 drivers/spi/Makefile                               |    1 +
 drivers/spi/spi-armada-3700.c                      | 1040 ++++++++++++++++++++
 6 files changed, 1116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
 create mode 100644 drivers/spi/spi-armada-3700.c

-- 
2.9.3

^ permalink raw reply

* [RFC v3 04/10] iommu: iommu_alloc_resv_region
From: Auger Eric @ 2016-11-30  9:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161129161128.GI2078@8bytes.org>

Hi Joerg,

On 29/11/2016 17:11, Joerg Roedel wrote:
> On Tue, Nov 15, 2016 at 01:09:17PM +0000, Eric Auger wrote:
>> +static inline struct iommu_resv_region *
>> +iommu_alloc_resv_region(phys_addr_t start, size_t length, unsigned int prot)
>> +{
>> +	return NULL;
>> +}
>> +
> 
> Will this function be called outside of iommu code?

No the function is not bound to be called outside of the iommu code. I
will remove this.

Thanks

Eric
> 
> 
> 
> 	Joerg
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply

* [RFC PATCH] ARM: dts: sun8i: add simplefb node for H3
From: Jean-Francois Moine @ 2016-11-30  9:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161129215932.kmk6qodthicadpyb@lukather>

On Tue, 29 Nov 2016 22:59:32 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:

> > > I'm still not sure which pipeline should I use.
> > > 
> > > And, it seems that HDMI Slow Clock is not needed?
> > > 
> > > (seems that it's only for EDID, but simplefb won't use EDID)
> > 
> > So, I don't see how this may work.
> > How can the u-boot know the resolutions of the HDMI display device?
> > 
> > In other words: I have a new H3 board with the last u-boot and kernel.
> > I plug my (rather old or brand new) HDMI display device.
> > After powering on the system, I hope to get something on the screen.
> > How?
> 
> If it works like the driver for the first display engine in U-Boot, it
> will use the preferred mode reported by the EDID, and will fallback to
> 1024x768 if it cannot access it.

Icenowy wrote: "simplefb won't use EDID"

Then, if it is like in the kernel, the 1024x768 mode is VGA. It does
not work with HDMI (different timings).

> Maybe it would be worth exchanging on the EDID code that has been done
> for the u-boot driver too, so that it can be fixed in your driver.

The u-boot got my code, and, up to now, I could not fix the random or
permanent failures of EDID reading in some boards.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Jean-Francois Moine @ 2016-11-30  9:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4614815.L3DQhhBy6d@avalon>

On Wed, 30 Nov 2016 10:20:21 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> > Well, I don't see what this connector can be.
> > May you give me a DT example?
> 
> Sure.
> 
> arch/arm/boot/dts/r8a7791-koelsch.dts
> 
>         /* HDMI encoder */
> 
>         hdmi at 39 {
>                 compatible = "adi,adv7511w";
>                 reg = <0x39>;
>                 interrupt-parent = <&gpio3>;
>                 interrupts = <29 IRQ_TYPE_LEVEL_LOW>;
> 
>                 adi,input-depth = <8>;
>                 adi,input-colorspace = "rgb";
>                 adi,input-clock = "1x";
>                 adi,input-style = <1>;
>                 adi,input-justification = "evenly";
> 
>                 ports {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         port at 0 {
>                                 reg = <0>;
>                                 adv7511_in: endpoint {
>                                         remote-endpoint = <&du_out_rgb>;
>                                 };
>                         };
> 
>                         port at 1 {
>                                 reg = <1>;
>                                 adv7511_out: endpoint {
>                                         remote-endpoint = <&hdmi_con>;
>                                 };
>                         };
>                 };
>         };
> 
>         /* HDMI connector */
> 
>         hdmi-out {
>                 compatible = "hdmi-connector";
>                 type = "a";
> 
>                 port {
>                         hdmi_con: endpoint {
>                                 remote-endpoint = <&adv7511_out>;
>                         };
>                 };
>         };

Hi Laurent,

Sorry for I don't see the interest:
- it is obvious that the HDMI connector is a 'hdmi-connector'!
- the physical connector type may be changed on any board by a soldering
  iron or a connector to connector cable.
- what does the software do with the connector type?
- why not to put the connector information in the HDMI device?

And, if I follow you, the graph of ports could also be used to describe
the way the various parts of the SoCs are powered, to describe the pin
connections, to describe the USB connectors, to describe the board
internal hubs and bridges...

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v3 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2016-11-30  9:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20160722070219.GB1605@katana>

Hi Wolfram,

Thanks for reviewing this driver and sorry for this quite long answer.
I was too busy in another project but now I am ready to complete the
upstream of the STM32F4 I2C driver.

2016-07-22 9:02 GMT+02:00 Wolfram Sang <wsa@the-dreams.de>:
> Hi,
>
> thanks for this contribution! Looks mostly good, some comments, though.
>
> On Mon, Jun 20, 2016 at 06:22:48PM +0200, M'boumba Cedric Madianga wrote:
>> This patch adds support for the STM32F4 I2C controller.
>>
>> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> ---
>>  drivers/i2c/busses/Kconfig       |  10 +
>>  drivers/i2c/busses/Makefile      |   1 +
>>  drivers/i2c/busses/i2c-stm32f4.c | 863 +++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 874 insertions(+)
>>  create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
>>
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index faa8e68..805acf6 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -873,6 +873,16 @@ config I2C_ST
>>         This driver can also be built as module. If so, the module
>>         will be called i2c-st.
>>
>> +config I2C_STM32F4
>> +     tristate "STMicroelectronics STM32F4 I2C support"
>> +     depends on ARCH_STM32
>
> || COMPILE_TEST?
Ok good point. I will fix it in the V4.

>
>> +     help
>> +       Enable this option to add support for STM32 I2C controller embedded
>> +       in STM32F4 SoCs.
>> +
>> +       This driver can also be built as module. If so, the module
>> +       will be called i2c-stm32f4.
>> +
>>  config I2C_STU300
>>       tristate "ST Microelectronics DDC I2C interface"
>>       depends on MACH_U300
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index 37f2819..2ac0eb2 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -84,6 +84,7 @@ obj-$(CONFIG_I2C_SH_MOBILE) += i2c-sh_mobile.o
>>  obj-$(CONFIG_I2C_SIMTEC)     += i2c-simtec.o
>>  obj-$(CONFIG_I2C_SIRF)               += i2c-sirf.o
>>  obj-$(CONFIG_I2C_ST)         += i2c-st.o
>> +obj-$(CONFIG_I2C_STM32F4)    += i2c-stm32f4.o
>>  obj-$(CONFIG_I2C_STU300)     += i2c-stu300.o
>>  obj-$(CONFIG_I2C_SUN6I_P2WI) += i2c-sun6i-p2wi.o
>>  obj-$(CONFIG_I2C_TEGRA)              += i2c-tegra.o
>> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
>> new file mode 100644
>> index 0000000..652d4bf
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-stm32f4.c
>> @@ -0,0 +1,863 @@
>> +/*
>> + * Driver for STMicroelectronics STM32 I2C controller
>> + *
>> + * Copyright (C) M'boumba Cedric Madianga 2015
>> + * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
>> + *
>> + * This driver is based on st-i2c.c
>
> i2c-st.c ?
You are right. Thanks.

>
>> + *
>> + * License terms:  GNU General Public License (GPL), version 2
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +
>> +/* STM32F4 I2C offset registers */
>> +#define STM32F4_I2C_CR1                      0x00
>> +#define STM32F4_I2C_CR2                      0x04
>> +#define STM32F4_I2C_DR                       0x10
>> +#define STM32F4_I2C_SR1                      0x14
>> +#define STM32F4_I2C_SR2                      0x18
>> +#define STM32F4_I2C_CCR                      0x1C
>> +#define STM32F4_I2C_TRISE            0x20
>> +#define STM32F4_I2C_FLTR             0x24
>> +
>> +/* STM32F4 I2C control 1*/
>> +#define STM32F4_I2C_CR1_SWRST                BIT(15)
>> +#define STM32F4_I2C_CR1_POS          BIT(11)
>> +#define STM32F4_I2C_CR1_ACK          BIT(10)
>> +#define STM32F4_I2C_CR1_STOP         BIT(9)
>> +#define STM32F4_I2C_CR1_START                BIT(8)
>> +#define STM32F4_I2C_CR1_PE           BIT(0)
>> +
>> +/* STM32F4 I2C control 2 */
>> +#define STM32F4_I2C_CR2_FREQ_MASK    GENMASK(5, 0)
>> +#define STM32F4_I2C_CR2_FREQ(n)              ((n & STM32F4_I2C_CR2_FREQ_MASK))
>> +#define STM32F4_I2C_CR2_ITBUFEN              BIT(10)
>> +#define STM32F4_I2C_CR2_ITEVTEN              BIT(9)
>> +#define STM32F4_I2C_CR2_ITERREN              BIT(8)
>> +#define STM32F4_I2C_CR2_IRQ_MASK     (STM32F4_I2C_CR2_ITBUFEN \
>> +                                     | STM32F4_I2C_CR2_ITEVTEN \
>> +                                     | STM32F4_I2C_CR2_ITERREN)
>> +
>> +/* STM32F4 I2C Status 1 */
>> +#define STM32F4_I2C_SR1_AF           BIT(10)
>> +#define STM32F4_I2C_SR1_ARLO         BIT(9)
>> +#define STM32F4_I2C_SR1_BERR         BIT(8)
>> +#define STM32F4_I2C_SR1_TXE          BIT(7)
>> +#define STM32F4_I2C_SR1_RXNE         BIT(6)
>> +#define STM32F4_I2C_SR1_BTF          BIT(2)
>> +#define STM32F4_I2C_SR1_ADDR         BIT(1)
>> +#define STM32F4_I2C_SR1_SB           BIT(0)
>> +#define STM32F4_I2C_SR1_ITEVTEN_MASK (STM32F4_I2C_SR1_BTF \
>> +                                     | STM32F4_I2C_SR1_ADDR \
>> +                                     | STM32F4_I2C_SR1_SB)
>> +#define STM32F4_I2C_SR1_ITBUFEN_MASK (STM32F4_I2C_SR1_TXE \
>> +                                     | STM32F4_I2C_SR1_RXNE)
>> +#define STM32F4_I2C_SR1_ITERREN_MASK (STM32F4_I2C_SR1_AF \
>> +                                     | STM32F4_I2C_SR1_ARLO \
>> +                                     | STM32F4_I2C_SR1_BERR)
>> +
>> +/* STM32F4 I2C Status 2 */
>> +#define STM32F4_I2C_SR2_BUSY         BIT(1)
>> +
>> +/* STM32F4 I2C Control Clock */
>> +#define STM32F4_I2C_CCR_CCR_MASK     GENMASK(11, 0)
>> +#define STM32F4_I2C_CCR_CCR(n)               ((n & STM32F4_I2C_CCR_CCR_MASK))
>> +#define STM32F4_I2C_CCR_FS           BIT(15)
>> +#define STM32F4_I2C_CCR_DUTY         BIT(14)
>> +
>> +/* STM32F4 I2C Trise */
>> +#define STM32F4_I2C_TRISE_VALUE_MASK GENMASK(5, 0)
>> +#define STM32F4_I2C_TRISE_VALUE(n)   ((n & STM32F4_I2C_TRISE_VALUE_MASK))
>> +
>> +/* STM32F4 I2C Filter */
>> +#define STM32F4_I2C_FLTR_DNF_MASK    GENMASK(3, 0)
>> +#define STM32F4_I2C_FLTR_DNF(n)              ((n & STM32F4_I2C_FLTR_DNF_MASK))
>> +#define STM32F4_I2C_FLTR_ANOFF               BIT(4)
>> +
>> +#define STM32F4_I2C_MIN_FREQ         2
>> +#define STM32F4_I2C_MAX_FREQ         42
>> +#define FAST_MODE_MAX_RISE_TIME              1000
>> +#define STD_MODE_MAX_RISE_TIME               300
>> +#define MHZ_TO_HZ                    1000000
>> +
>> +enum stm32f4_i2c_speed {
>> +     STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
>> +     STM32F4_I2C_SPEED_FAST, /* 400 kHz */
>> +     STM32F4_I2C_SPEED_END,
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_timings - per-Mode tuning parameters
>> + * @duty: Fast mode duty cycle
>> + * @mul_ccr: Value to be multiplied to CCR to reach 100Khz/400Khz SCL frequency
>> + * @min_ccr: Minimum clock ctrl reg value to reach 100Khz/400Khz SCL frequency
>> + */
>> +struct stm32f4_i2c_timings {
>> +     u32 rate;
>> +     u32 duty;
>> +     u32 mul_ccr;
>> +     u32 min_ccr;
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_client - client specific data
>> + * @addr: 8-bit slave addr, including r/w bit
>> + * @count: number of bytes to be transferred
>> + * @buf: data buffer
>> + * @result: result of the transfer
>> + * @stop: last I2C msg to be sent, i.e. STOP to be generated
>> + */
>> +struct stm32f4_i2c_client {
>
> I think the name is a little misleading. Check 'struct i2c_client' as a
> comparison. A more comprehensible name might be stm32f4_i2c_msg...
Ok. I will fix it in the V4.

>
>> +     u8      addr;
>> +     u32     count;
>> +     u8      *buf;
>> +     int     result;
>> +     bool    stop;
>> +};
>> +
>> +/**
>> + * struct stm32f4_i2c_dev - private data of the controller
>> + * @adap: I2C adapter for this controller
>> + * @dev: device for this controller
>> + * @base: virtual memory area
>> + * @complete: completion of I2C message
>> + * @irq_event: interrupt event line for the controller
>> + * @irq_error: interrupt error line for the controller
>> + * @clk: hw i2c clock
>> + * speed: I2C clock frequency of the controller. Standard or Fast only supported
>> + * @client: I2C transfer information
>> + * @rst: I2C reset line
>> + */
>> +struct stm32f4_i2c_dev {
>> +     struct i2c_adapter              adap;
>> +     struct device                   *dev;
>> +     void __iomem                    *base;
>> +     struct completion               complete;
>> +     int                             irq_event;
>> +     int                             irq_error;
>> +     struct clk                      *clk;
>> +     int                             speed;
>> +     struct stm32f4_i2c_client       client;
>> +     struct reset_control            *rst;
>
> No need to store reset_control. You just use it in probe.
OK. Thanks.

>
>> +};
>> +
>> +static struct stm32f4_i2c_timings i2c_timings[] = {
>> +     [STM32F4_I2C_SPEED_STANDARD] = {
>> +             .mul_ccr                = 1,
>> +             .min_ccr                = 4,
>> +             .duty                   = 0,
>> +     },
>> +     [STM32F4_I2C_SPEED_FAST] = {
>> +             .mul_ccr                = 16,
>> +             .min_ccr                = 1,
>> +             .duty                   = 1,
>> +     },
>> +};
>> +
>> +static inline void stm32f4_i2c_set_bits(void __iomem *reg, u32 mask)
>> +{
>> +     writel_relaxed(readl_relaxed(reg) | mask, reg);
>> +}
>> +
>> +static inline void stm32f4_i2c_clr_bits(void __iomem *reg, u32 mask)
>> +{
>> +     writel_relaxed(readl_relaxed(reg) & ~mask, reg);
>> +}
>> +
>> +static void stm32f4_i2c_soft_reset(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +
>> +     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);
>> +}
>> +
>> +static void stm32f4_i2c_disable_it(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
>> +}
>> +
>> +static void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 clk_rate, cr2, freq;
>> +
>> +     cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
>> +
>> +     clk_rate = clk_get_rate(i2c_dev->clk);
>> +     freq = clk_rate / MHZ_TO_HZ;
>> +
>> +     if (freq > STM32F4_I2C_MAX_FREQ)
>> +             freq = STM32F4_I2C_MAX_FREQ;
>> +     if (freq < STM32F4_I2C_MIN_FREQ)
>> +             freq = STM32F4_I2C_MIN_FREQ;
>
> clamp() to enforce the range?
Sorry but what do you mean by "clamp()" ?

>
>> +
>> +     cr2 |= STM32F4_I2C_CR2_FREQ(freq);
>> +     writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);
>> +}
>> +
>> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 trise, freq, cr2, val;
>> +
>> +     cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
>> +
>> +     trise = readl_relaxed(i2c_dev->base + STM32F4_I2C_TRISE);
>> +     trise &= ~STM32F4_I2C_TRISE_VALUE_MASK;
>> +
>> +     /* Maximum rise time computation */
>> +     if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
>> +             trise |= STM32F4_I2C_TRISE_VALUE((freq + 1));
>> +     } else {
>> +             val = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;
>> +             trise |= STM32F4_I2C_TRISE_VALUE((val + 1));
>> +     }
>> +
>> +     writel_relaxed(trise, i2c_dev->base + STM32F4_I2C_TRISE);
>> +}
>> +
>> +static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
>> +     u32 ccr, clk_rate;
>> +     int val;
>> +
>> +     ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
>> +     ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
>> +              STM32F4_I2C_CCR_CCR_MASK);
>> +
>> +     clk_rate = clk_get_rate(i2c_dev->clk);
>> +     val = clk_rate / MHZ_TO_HZ * t->mul_ccr;
>> +     if (val < t->min_ccr)
>> +             val = t->min_ccr;
>> +     ccr |= STM32F4_I2C_CCR_CCR(val);
>> +
>> +     if (t->duty)
>> +             ccr |= STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY;
>> +
>> +     writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
>> +}
>> +
>> +static void stm32f4_i2c_set_filter(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 filter;
>> +
>> +     /* Enable analog noise filter and disable digital noise filter */
>> +     filter = readl_relaxed(i2c_dev->base + STM32F4_I2C_FLTR);
>> +     filter &= ~(STM32F4_I2C_FLTR_ANOFF | STM32F4_I2C_FLTR_DNF_MASK);
>> +     writel_relaxed(filter, i2c_dev->base + STM32F4_I2C_FLTR);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_hw_config() - Prepare I2C block
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +
>> +     /* Disable I2C */
>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
>> +
>> +     stm32f4_i2c_set_periph_clk_freq(i2c_dev);
>> +
>> +     stm32f4_i2c_set_rise_time(i2c_dev);
>> +
>> +     stm32f4_i2c_set_speed_mode(i2c_dev);
>> +
>> +     stm32f4_i2c_set_filter(i2c_dev);
>> +
>> +     /* Enable I2C */
>> +     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
>> +}
>> +
>> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     u32 status;
>> +     int ret;
>> +
>> +     ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
>> +                                      status,
>> +                                      !(status & STM32F4_I2C_SR2_BUSY),
>> +                                      10, 1000);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "bus not free\n");
>> +             ret = -EBUSY;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_write_ byte() - Write a byte in the data register
>> + * @i2c_dev: Controller's private data
>> + * @byte: Data to write in the register
>> + */
>> +static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
>> +{
>> +     writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_write_msg() - Fill the data register in write mode
>> + * @i2c_dev: Controller's private data
>> + *
>> + * This function fills the data register with I2C transfer buffer
>> + */
>> +static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +
>> +     stm32f4_i2c_write_byte(i2c_dev, *c->buf++);
>> +     c->count--;
>> +}
>> +
>> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     u32 rbuf;
>> +
>> +     rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
>> +     *c->buf++ = (u8)rbuf & 0xff;
>> +     c->count--;
>> +}
>> +
>> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     stm32f4_i2c_disable_it(i2c_dev);
>> +
>> +     reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +     if (c->stop)
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +     else
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +
>> +     complete(&i2c_dev->complete);
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     if (c->count) {
>> +             stm32f4_i2c_write_msg(i2c_dev);
>> +             if (!c->count) {
>> +                     /* Disable BUF interrupt */
>> +                     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> +             }
>> +     } else {
>> +             stm32f4_i2c_terminate_xfer(i2c_dev);
>> +     }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +
>> +     switch (c->count) {
>> +     case 1:
>> +             stm32f4_i2c_disable_it(i2c_dev);
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +             complete(&i2c_dev->complete);
>> +             break;
>> +     case 2:
>> +     case 3:
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> +             break;
>> +     default:
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +     }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
>> + * in case of read
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg;
>> +     u32 mask;
>> +     int i;
>> +
>> +     switch (c->count) {
>> +     case 2:
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             /* Generate STOP or REPSTART */
>> +             if (c->stop)
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +             else
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +
>> +             /* Read two last data bytes */
>> +             for (i = 2; i > 0; i--)
>> +                     stm32f4_i2c_read_msg(i2c_dev);
>> +
>> +             /* Disable EVT and ERR interrupt */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +             mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>> +             stm32f4_i2c_clr_bits(reg, mask);
>> +
>> +             complete(&i2c_dev->complete);
>> +             break;
>> +     case 3:
>> +             /* Enable ACK and read data */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +             break;
>> +     default:
>> +             stm32f4_i2c_read_msg(i2c_dev);
>> +     }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
>> + * master receiver
>> + * @i2c_dev: Controller's private data
>> + */
>> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg;
>> +     u32 sr2;
>> +
>> +     switch (c->count) {
>> +     case 0:
>> +             stm32f4_i2c_terminate_xfer(i2c_dev);
>> +             /* Clear ADDR flag */
>> +             sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             break;
>> +     case 1:
>> +             /*
>> +              * Single byte reception:
>> +              * Enable NACK, clear ADDR flag and generate STOP or RepSTART
>> +              */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             if (c->stop)
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +             else
>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +             break;
>> +     case 2:
>> +             /*
>> +              * 2-byte reception:
>> +              * Enable NACK and PEC Position Ack and clear ADDR flag
>> +              */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>> +             sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             break;
>> +
>> +     default:
>> +             /* N-byte reception: Enable ACK and clear ADDR flag */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +             break;
>> +     }
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_isr_event() - Interrupt routine for I2C bus event
>> + * @irq: interrupt number
>> + * @data: Controller's private data
>> + */
>> +static irqreturn_t stm32f4_i2c_isr_event(int irq, void *data)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = data;
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg;
>> +     u32 real_status, possible_status, ien, sr2;
>> +     int flag;
>> +
>> +     ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     ien &= STM32F4_I2C_CR2_IRQ_MASK;
>> +     possible_status = 0;
>> +
>> +     /* Check possible status combinations */
>> +     if (ien & STM32F4_I2C_CR2_ITEVTEN) {
>> +             possible_status = STM32F4_I2C_SR1_ITEVTEN_MASK;
>> +             if (ien & STM32F4_I2C_CR2_ITBUFEN)
>> +                     possible_status |= STM32F4_I2C_SR1_ITBUFEN_MASK;
>> +     }
>> +
>> +     real_status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
>> +
>> +     if (!(real_status & possible_status)) {
>> +             dev_dbg(i2c_dev->dev,
>> +                     "spurious evt it (status=0x%08x, ien=0x%08x)\n",
>> +                     real_status, ien);
>> +             return IRQ_NONE;
>> +     }
>> +
>> +     /* Use __fls() to check error bits first */
>> +     flag = __fls(real_status & possible_status);
>> +
>> +     switch (1 << flag) {
>> +     case STM32F4_I2C_SR1_SB:
>> +             stm32f4_i2c_write_byte(i2c_dev, c->addr);
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_ADDR:
>> +             if (c->addr & I2C_M_RD)
>> +                     stm32f4_i2c_handle_rx_addr(i2c_dev);
>> +             else
>> +                     sr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>> +
>> +             /* Enable ITBUF interrupts */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR2;
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_BTF:
>> +             if (c->addr & I2C_M_RD)
>> +                     stm32f4_i2c_handle_rx_btf(i2c_dev);
>> +             else
>> +                     stm32f4_i2c_handle_write(i2c_dev);
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_TXE:
>> +             stm32f4_i2c_handle_write(i2c_dev);
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_RXNE:
>> +             stm32f4_i2c_handle_read(i2c_dev);
>> +             break;
>> +
>> +     default:
>> +             dev_err(i2c_dev->dev,
>> +                     "evt it unhandled: status=0x%08x)\n", real_status);
>> +             return IRQ_NONE;
>> +     }
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_isr_error() - Interrupt routine for I2C bus error
>> + * @irq: interrupt number
>> + * @data: Controller's private data
>> + */
>> +static irqreturn_t stm32f4_i2c_isr_error(int irq, void *data)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = data;
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg;
>> +     u32 real_status, possible_status, ien;
>> +     int flag;
>> +
>> +     ien = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
>> +     ien &= STM32F4_I2C_CR2_IRQ_MASK;
>> +     possible_status = 0;
>> +
>> +     /* Check possible status combinations */
>> +     if (ien & STM32F4_I2C_CR2_ITERREN)
>> +             possible_status = STM32F4_I2C_SR1_ITERREN_MASK;
>> +
>> +     real_status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);
>> +
>> +     if (!(real_status & possible_status)) {
>> +             dev_dbg(i2c_dev->dev,
>> +                     "spurious err it (status=0x%08x, ien=0x%08x)\n",
>> +                     real_status, ien);
>> +             return IRQ_NONE;
>> +     }
>> +
>> +     /* Use __fls() to check error bits first */
>> +     flag = __fls(real_status & possible_status);
>> +
>> +     switch (1 << flag) {
>> +     case STM32F4_I2C_SR1_BERR:
>> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_BERR);
>> +             c->result = -EIO;
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_ARLO:
>> +             reg = i2c_dev->base + STM32F4_I2C_SR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_SR1_ARLO);
>> +             c->result = -EAGAIN;
>> +             break;
>> +
>> +     case STM32F4_I2C_SR1_AF:
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>> +             c->result = -EIO;
>> +             break;
>> +
>> +     default:
>> +             dev_err(i2c_dev->dev,
>> +                     "err it unhandled: status=0x%08x)\n", real_status);
>> +             return IRQ_NONE;
>> +     }
>> +
>> +     stm32f4_i2c_soft_reset(i2c_dev);
>> +     stm32f4_i2c_disable_it(i2c_dev);
>> +     complete(&i2c_dev->complete);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_xfer_msg() - Transfer a single I2C message
>> + * @i2c_dev: Controller's private data
>> + * @msg: I2C message to transfer
>> + * @is_first: first message of the sequence
>> + * @is_last: last message of the sequence
>> + */
>> +static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
>> +                             struct i2c_msg *msg, bool is_first,
>> +                             bool is_last)
>> +{
>> +     struct stm32f4_i2c_client *c = &i2c_dev->client;
>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +     unsigned long timeout;
>> +     u32 mask;
>> +     int ret;
>> +
>> +     c->addr = (u8)(msg->addr << 1);
>> +     c->addr |= (msg->flags & I2C_M_RD);
>
> Use the shiny new "i2c_8bit_addr_from_msg()" for the last two lines.
Good. Thanks. I will use it in the V4.

>
>> +     c->buf = msg->buf;
>> +     c->count = msg->len;
>> +     c->result = 0;
>> +     c->stop = is_last;
>> +
>> +     reinit_completion(&i2c_dev->complete);
>> +
>> +     /* Enable ITEVT and ITERR interrupts */
>> +     mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>> +     stm32f4_i2c_set_bits(i2c_dev->base + STM32F4_I2C_CR2, mask);
>> +
>> +     if (is_first) {
>> +             ret = stm32f4_i2c_wait_free_bus(i2c_dev);
>> +             if (ret)
>> +                     return ret;
>> +
>> +             /* START generation */
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>> +     }
>> +
>> +     timeout = wait_for_completion_timeout(&i2c_dev->complete,
>> +                                           i2c_dev->adap.timeout);
>> +     ret = c->result;
>> +
>> +     /* Disable PEC position Ack */
>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_POS);
>> +
>> +     if (!timeout) {
>> +             dev_err(i2c_dev->dev, "Access to slave 0x%x timed out\n",
>> +                     c->addr >> 1);
>
> I don't recommend writing err messages for timeouts. They regularly
> happen, e.g. when an eeprom is doing a page write cycle.
OK

>
>> +             ret = -ETIMEDOUT;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * stm32f4_i2c_xfer() - Transfer combined I2C message
>> + * @i2c_adap: Adapter pointer to the controller
>> + * @msgs: Pointer to data to be written.
>> + * @num: Number of messages to be executed
>> + */
>> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[],
>> +                         int num)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
>> +     int ret, i;
>> +
>> +     ret = clk_enable(i2c_dev->clk);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "Failed to enable clock\n");
>> +             return ret;
>> +     }
>> +
>> +     stm32f4_i2c_hw_config(i2c_dev);
>> +
>> +     for (i = 0; i < num && !ret; i++)
>> +             ret = stm32f4_i2c_xfer_msg(i2c_dev, &msgs[i], i == 0,
>> +                                        i == num - 1);
>> +
>> +     clk_disable(i2c_dev->clk);
>> +
>> +     return (ret < 0) ? ret : i;
>> +}
>> +
>> +static u32 stm32f4_i2c_func(struct i2c_adapter *adap)
>> +{
>> +     return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
>> +}
>> +
>> +static struct i2c_algorithm stm32f4_i2c_algo = {
>> +     .master_xfer = stm32f4_i2c_xfer,
>> +     .functionality = stm32f4_i2c_func,
>> +};
>> +
>> +static int stm32f4_i2c_probe(struct platform_device *pdev)
>> +{
>> +     struct device_node *np = pdev->dev.of_node;
>> +     struct stm32f4_i2c_dev *i2c_dev;
>> +     struct resource *res;
>> +     u32 clk_rate;
>> +     struct i2c_adapter *adap;
>> +     int ret;
>> +
>> +     i2c_dev = devm_kzalloc(&pdev->dev, sizeof(*i2c_dev), GFP_KERNEL);
>> +     if (!i2c_dev)
>> +             return -ENOMEM;
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     i2c_dev->base = devm_ioremap_resource(&pdev->dev, res);
>> +     if (IS_ERR(i2c_dev->base))
>> +             return PTR_ERR(i2c_dev->base);
>> +
>> +     i2c_dev->irq_event = irq_of_parse_and_map(np, 0);
>> +     if (!i2c_dev->irq_event) {
>> +             dev_err(&pdev->dev, "IRQ missing or invalid\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     i2c_dev->irq_error = irq_of_parse_and_map(np, 1);
>> +     if (!i2c_dev->irq_error) {
>> +             dev_err(&pdev->dev, "IRQ missing or invalid\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     i2c_dev->clk = devm_clk_get(&pdev->dev, NULL);
>> +     if (IS_ERR(i2c_dev->clk)) {
>> +             dev_err(&pdev->dev, "Error: Missing controller clock\n");
>> +             return PTR_ERR(i2c_dev->clk);
>> +     }
>> +     ret = clk_prepare(i2c_dev->clk);
>> +     if (ret) {
>> +             dev_err(i2c_dev->dev, "Failed to prepare clock\n");
>> +             return ret;
>> +     }
>> +
>> +     i2c_dev->rst = devm_reset_control_get(&pdev->dev, NULL);
>> +     if (IS_ERR(i2c_dev->rst)) {
>> +             dev_err(&pdev->dev, "Error: Missing controller reset\n");
>> +             return PTR_ERR(i2c_dev->rst);
>> +     }
>> +     reset_control_assert(i2c_dev->rst);
>> +     udelay(2);
>> +     reset_control_deassert(i2c_dev->rst);
>> +
>> +     i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
>> +     ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
>> +     if ((!ret) && (clk_rate == 400000))
>> +             i2c_dev->speed = STM32F4_I2C_SPEED_FAST;
>> +
>> +     i2c_dev->dev = &pdev->dev;
>> +
>> +     ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_event,
>> +                                     NULL, stm32f4_i2c_isr_event,
>> +                                     IRQF_ONESHOT, pdev->name, i2c_dev);
>> +     if (ret) {
>> +             dev_err(&pdev->dev, "Failed to request irq %i\n",
>> +                     i2c_dev->irq_error);
>> +             goto clk_free;
>> +     }
>> +
>> +     ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_error,
>> +                                     NULL, stm32f4_i2c_isr_error,
>> +                                     IRQF_ONESHOT, pdev->name, i2c_dev);
>> +     if (ret) {
>> +             dev_err(&pdev->dev, "Failed to request irq %i\n",
>> +                     i2c_dev->irq_error);
>> +             goto clk_free;
>> +     }
>> +
>> +     adap = &i2c_dev->adap;
>> +     i2c_set_adapdata(adap, i2c_dev);
>> +     snprintf(adap->name, sizeof(adap->name), "STM32 I2C(%pa)", &res->start);
>> +     adap->owner = THIS_MODULE;
>> +     adap->timeout = 2 * HZ;
>> +     adap->retries = 0;
>> +     adap->algo = &stm32f4_i2c_algo;
>> +     adap->dev.parent = &pdev->dev;
>> +     adap->dev.of_node = pdev->dev.of_node;
>> +
>> +     init_completion(&i2c_dev->complete);
>> +
>> +     ret = i2c_add_adapter(adap);
>> +     if (ret) {
>> +             dev_err(&pdev->dev, "Failed to add adapter\n");
>
> Please drop this messages. We have patches in i2c/for-next where the
> core will report all errors.
OK

>
>> +             goto clk_free;
>> +     }
>> +
>> +     platform_set_drvdata(pdev, i2c_dev);
>> +
>> +     dev_info(i2c_dev->dev, "STM32F4 I2C driver initialized\n");
>> +
>> +     return 0;
>> +
>> +clk_free:
>> +     clk_unprepare(i2c_dev->clk);
>> +     return ret;
>> +}
>> +
>> +static int stm32f4_i2c_remove(struct platform_device *pdev)
>> +{
>> +     struct stm32f4_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
>> +
>> +     i2c_del_adapter(&i2c_dev->adap);
>> +
>> +     clk_unprepare(i2c_dev->clk);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct of_device_id stm32f4_i2c_match[] = {
>> +     { .compatible = "st,stm32f4-i2c", },
> ^> +    {},
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32f4_i2c_match);
>> +
>> +static struct platform_driver stm32f4_i2c_driver = {
>> +     .driver = {
>> +             .name = "stm32f4-i2c",
>> +             .of_match_table = stm32f4_i2c_match,
>> +     },
>> +     .probe = stm32f4_i2c_probe,
>> +     .remove = stm32f4_i2c_remove,
>> +};
>> +
>> +module_platform_driver(stm32f4_i2c_driver);
>> +
>> +MODULE_AUTHOR("M'boumba Cedric Madianga <cedric.madianga@gmail.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics STM32F4 I2C driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 1.9.1
>>

^ permalink raw reply

* [PATCH 3/3] ARM: dts: sunxi: enable SDIO Wi-Fi on Orange Pi Zero
From: Andre Przywara @ 2016-11-30  9:25 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161129131922.JFbeipav@smtp2o.mail.yandex.net>

Hi,

On 29/11/16 10:19, Icenowy Zheng wrote:
> 
> 2016?11?29? 15:16? Alexey Kardashevskiy <aik@ozlabs.ru>???
>>
>>
>>
>> On Wed, Nov 23, 2016 at 6:59 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Nov 22, 2016 at 12:24:21AM +0800, Icenowy Zheng wrote:
>>> > There's a Allwinner's XR819 SDIO Wi-Fi module soldered on the board of
>>> > Orange Pi Zero, which used a dedicated regulator to power.
>>> >
>>> > Add the device tree node of the regulator, the enable gpio (with
>>> > mmc-pwrseq) and the sdio controller.
>>> >
>>> > There's a out-of-tree driver tested to work with this device tree.
>>
>>
>> btw could you please give a pointer where to find a XR819 driver for
> relatively recent kernel (4.8 may be, just not 3.4)? Thanks.
> 
> https://github.com/Icenowy/xradio

I was just curious, so pulled your tree and tried to just compile it. It
still threw warnings at me for ARM, and even more so for arm64.
I fixed all of them and put that on my github[1]. Feel free to just pick
them from there or wait till I manage to clean them up and send you a
pull request.
And also just a a test, I quickly put it in drivers/net/wireless/xradio,
where it compiled fine after registering it with the upper level Kconfig
and Makefile.

And while looking at it: This looks like typical AW code, not even
remotely upstreameable and probably far too complicated. Enabling some
Kconfig options made it complain about missing functions.
Has anyone checked if this is close to an existing WiFi chip? That
wouldn't be a first ;-)

Cheers,
Andre.

[1] https://github.com/apritzel/xradio/commits/quickfixes

^ permalink raw reply

* [LINUX RFC v4 3/4] mtd: spi-nor: add stripe support
From: Naga Sureshkumar Relli @ 2016-11-30  9:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <68ca0f19-e534-3361-11f0-6566626380fe@atmel.com>

Hi Cyrille,

> I have not finished to review the whole series yet but here some first
> comments:

Thanks for reviewing these patch series.

> 
> Le 27/11/2016 ? 09:33, Naga Sureshkumar Relli a ?crit :
> > This patch adds stripe support and it is needed for GQSPI parallel
> > configuration mode by:
> >
> > - Adding required parameters like stripe and shift to spi_nor
> >   structure.
> > - Initializing all added parameters in spi_nor_scan()
> > - Updating read_sr() and read_fsr() for getting status from both
> >   flashes
> > - Increasing page_size, sector_size, erase_size and toatal flash
> >   size as and when required.
> > - Dividing address by 2
> > - Updating spi->master->flags for qspi driver to change CS
> >
> > Signed-off-by: Naga Sureshkumar Relli <nagasure@xilinx.com>
> > ---
> > Changes for v4:
> >  - rename isparallel to stripe
> > Changes for v3:
> >  - No change
> > Changes for v2:
> >  - Splitted to separate MTD layer changes from SPI core changes
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 130
> ++++++++++++++++++++++++++++++++----------
> >  include/linux/mtd/spi-nor.h   |   2 +
> >  2 files changed, 103 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..4252239 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/of_platform.h>
> >  #include <linux/spi/flash.h>
> >  #include <linux/mtd/spi-nor.h>
> > +#include <linux/spi/spi.h>
> >
> >  /* Define max times to check status register before we give up. */
> >
> > @@ -89,15 +90,24 @@ static const struct flash_info
> > *spi_nor_match_id(const char *name);  static int read_sr(struct
> > spi_nor *nor)  {
> >  	int ret;
> > -	u8 val;
> > +	u8 val[2];
> >
> > -	ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val, 1);
> > -	if (ret < 0) {
> > -		pr_err("error %d reading SR\n", (int) ret);
> > -		return ret;
> > +	if (nor->stripe) {
> > +		ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 2);
> > +		if (ret < 0) {
> > +			pr_err("error %d reading SR\n", (int) ret);
> > +			return ret;
> > +		}
> > +		val[0] |= val[1];
> Why '|' rather than '&' ?
> I guess because of the 'Write In Progress/Busy' bit: when called by
> spi_nor_sr_ready(), you want to be sure that this 'BUSY' bit is cleared on
> both memories.
> 
> But what about when the Status Register is read for purpose other than
> checking the state of the 'BUSY' bit?
> 
Yes you are correct, I will change this.

> What about SPI controllers supporting more than 2 memories in parallel?
> 
> This solution might fit the ZynqMP controller but doesn't look so generic.
> 
> > +	} else {
> > +		ret = nor->read_reg(nor, SPINOR_OP_RDSR, &val[0], 1);
> > +		if (ret < 0) {
> > +			pr_err("error %d reading SR\n", (int) ret);
> > +			return ret;
> > +		}
> >  	}
> >
> > -	return val;
> > +	return val[0];
> >  }
> >
> >  /*
> > @@ -108,15 +118,24 @@ static int read_sr(struct spi_nor *nor)  static
> > int read_fsr(struct spi_nor *nor)  {
> >  	int ret;
> > -	u8 val;
> > +	u8 val[2];
> >
> > -	ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val, 1);
> > -	if (ret < 0) {
> > -		pr_err("error %d reading FSR\n", ret);
> > -		return ret;
> > +	if (nor->stripe) {
> > +		ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 2);
> > +		if (ret < 0) {
> > +			pr_err("error %d reading FSR\n", ret);
> > +			return ret;
> > +		}
> > +		val[0] &= val[1];
> Same comment here: why '&' rather than '|'?
> Surely because of the the 'READY' bit which should be set for both memories.
I will update this also.
> 
> > +	} else {
> > +		ret = nor->read_reg(nor, SPINOR_OP_RDFSR, &val[0], 1);
> > +		if (ret < 0) {
> > +			pr_err("error %d reading FSR\n", ret);
> > +			return ret;
> > +		}
> >  	}
> >
> > -	return val;
> > +	return val[0];
> >  }
> >
> >  /*
> > @@ -290,9 +309,16 @@ static int spi_nor_wait_till_ready(struct spi_nor
> *nor)
> >   */
> >  static int erase_chip(struct spi_nor *nor)  {
> > +	u32 ret;
> > +
> >  	dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
> >
> > -	return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
> > +	ret = nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return ret;
> > +
> 
> 	if (ret)
> 		return ret;
> 	else
> 		return ret;
> 
> This chunk should be removed, it doesn't ease the patch review ;)
Ok, I will remove.
> 
> >  }
> >
> >  static int spi_nor_lock_and_prep(struct spi_nor *nor, enum
> > spi_nor_ops ops) @@ -349,7 +375,7 @@ static int
> > spi_nor_erase_sector(struct spi_nor *nor, u32 addr)  static int
> > spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)  {
> >  	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> > -	u32 addr, len;
> > +	u32 addr, len, offset;
> >  	uint32_t rem;
> >  	int ret;
> >
> > @@ -399,9 +425,13 @@ static int spi_nor_erase(struct mtd_info *mtd,
> struct erase_info *instr)
> >  	/* "sector"-at-a-time erase */
> >  	} else {
> >  		while (len) {
> > +
> >  			write_enable(nor);
> > +			offset = addr;
> > +			if (nor->stripe)
> > +				offset /= 2;
> 
> I guess this should be /= 4 for controllers supporting 4 memories in parallel.
> Shouldn't you use nor->shift and define shift as an unsigned int instead of a
> bool?
> offset >>= nor->shift;
> 
Yes we can use this shift, I will update

> Anyway, by tuning the address here in spi-nor.c rather than in the SPI
> controller driver, you impose a model to support parallel memories that
> might not be suited to other controllers.

For this ZynqMP GQSPI controller parallel configuration, globally spi-nor should know about this stripe feature
And based on that address has to change.
As I mentioned in cover letter, this controller in parallel configuration will work with even addresses only.
i.e. Before creating address format(m25p_addr2cmd) in mtd layer, spi-nor should change that address based on stripe option.

I am updating this offset based on stripe option, and stripe option will update by reading dt property in nor_scan().
So the controller which doesn't support, then the stripe will be zero.
Or Can you please suggest any other way?

> 
> >
> > -			ret = spi_nor_erase_sector(nor, addr);
> > +			ret = spi_nor_erase_sector(nor, offset);
> >  			if (ret)
> >  				goto erase_err;
> >
> > @@ -525,6 +555,8 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs,
> uint64_t len)
> >  	bool use_top;
> >  	int ret;
> >
> > +	ofs = ofs >> nor->shift;
> > +
> >  	status_old = read_sr(nor);
> >  	if (status_old < 0)
> >  		return status_old;
> > @@ -610,6 +642,8 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs,
> uint64_t len)
> >  	bool use_top;
> >  	int ret;
> >
> > +	ofs = ofs >> nor->shift;
> > +
> >  	status_old = read_sr(nor);
> >  	if (status_old < 0)
> >  		return status_old;
> > @@ -709,6 +743,8 @@ static int spi_nor_lock(struct mtd_info *mtd, loff_t
> ofs, uint64_t len)
> >  	if (ret)
> >  		return ret;
> >
> > +	ofs = ofs >> nor->shift;
> > +
> >  	ret = nor->flash_lock(nor, ofs, len);
> >
> >  	spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_UNLOCK); @@ -
> 724,6 +760,8
> > @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t
> len)
> >  	if (ret)
> >  		return ret;
> >
> > +	ofs = ofs >> nor->shift;
> > +
> >  	ret = nor->flash_unlock(nor, ofs, len);
> >
> >  	spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK); @@ -
> 1018,6 +1056,9
> > @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
> >  	u8			id[SPI_NOR_MAX_ID_LEN];
> >  	const struct flash_info	*info;
> >
> > +	nor->spi->master->flags &= ~(SPI_MASTER_BOTH_CS |
> > +					SPI_MASTER_DATA_STRIPE);
> > +
> >  	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> SPI_NOR_MAX_ID_LEN);
> >  	if (tmp < 0) {
> >  		dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> @@ -1041,6
> > +1082,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from,
> > size_t len,  {
> >  	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> >  	int ret;
> > +	u32 offset = from;
> >
> >  	dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
> >
> > @@ -1049,7 +1091,13 @@ static int spi_nor_read(struct mtd_info *mtd,
> loff_t from, size_t len,
> >  		return ret;
> >
> >  	while (len) {
> > -		ret = nor->read(nor, from, len, buf);
> > +
> > +		offset = from;
> > +
> > +		if (nor->stripe)
> > +			offset /= 2;
> > +
> > +		ret = nor->read(nor, offset, len, buf);
> >  		if (ret == 0) {
> >  			/* We shouldn't see 0-length reads */
> >  			ret = -EIO;
> > @@ -1161,6 +1209,7 @@ static int spi_nor_write(struct mtd_info *mtd,
> loff_t to, size_t len,
> >  	struct spi_nor *nor = mtd_to_spi_nor(mtd);
> >  	size_t page_offset, page_remain, i;
> >  	ssize_t ret;
> > +	u32 offset;
> >
> >  	dev_dbg(nor->dev, "to 0x%08x, len %zd\n", (u32)to, len);
> >
> > @@ -1178,9 +1227,13 @@ static int spi_nor_write(struct mtd_info *mtd,
> loff_t to, size_t len,
> >  		/* the size of data remaining on the first page */
> >  		page_remain = min_t(size_t,
> >  				    nor->page_size - page_offset, len - i);
> > +		offset = (to + i);
> > +
> > +		if (nor->stripe)
> > +			offset /= 2;
> >
> >  		write_enable(nor);
> > -		ret = nor->write(nor, to + i, page_remain, buf + i);
> > +		ret = nor->write(nor, (offset), page_remain, buf + i);
> >  		if (ret < 0)
> >  			goto write_err;
> >  		written = ret;
> > @@ -1302,22 +1355,22 @@ static int spi_nor_check(struct spi_nor *nor)
> >
> >  int spi_nor_scan(struct spi_nor *nor, const char *name, enum
> > read_mode mode)  {
> > -	const struct flash_info *info = NULL;
> > +	struct flash_info *info = NULL;
> 
> You should not remove the const and should not try to modify members of
> *info.
> 
> >  	struct device *dev = nor->dev;
> >  	struct mtd_info *mtd = &nor->mtd;
> >  	struct device_node *np = spi_nor_get_flash_node(nor);
> > -	int ret;
> > -	int i;
> > +	struct device_node *np_spi;
> > +	int ret, i, xlnx_qspi_mode;
> >
> >  	ret = spi_nor_check(nor);
> >  	if (ret)
> >  		return ret;
> >
> >  	if (name)
> > -		info = spi_nor_match_id(name);
> > +		info = (struct flash_info *)spi_nor_match_id(name);
> >  	/* Try to auto-detect if chip name wasn't specified or not found */
> >  	if (!info)
> > -		info = spi_nor_read_id(nor);
> > +		info = (struct flash_info *)spi_nor_read_id(nor);
> >  	if (IS_ERR_OR_NULL(info))
> >  		return -ENOENT;
> >
> Both spi_nor_match_id() and spi_nor_read_id(), when they succeed, return
> a pointer to an entry of the spi_nor_ids[] array, which is located in a read-
> only memory area.
> 
> Since your patch doesn't remove the const attribute of the spi_nor_ids[], I
> wonder whether it has been tested. I expect it not to work on most
> architecture.
> 
> Anyway spi_nor_ids[] should remain const. Let's take the case of eXecution
> In Place (XIP) from an external memory: if spi_nor_ids[] is const, it can be
> read directly from this external (read-only) memory and we never need to
> copy the array in RAM, so we save some KB of RAM.
> This is just an example but I guess we can find other reasons to keep this
> array const.
> 
> > @@ -1341,7 +1394,7 @@ int spi_nor_scan(struct spi_nor *nor, const char
> *name, enum read_mode mode)
> >  			 */
> >  			dev_warn(dev, "found %s, expected %s\n",
> >  				 jinfo->name, info->name);
> > -			info = jinfo;
> > +			info = (struct flash_info *)jinfo;
> >  		}
> >  	}
> >
> > @@ -1370,6 +1423,27 @@ int spi_nor_scan(struct spi_nor *nor, const char
> *name, enum read_mode mode)
> >  	mtd->size = info->sector_size * info->n_sectors;
> >  	mtd->_erase = spi_nor_erase;
> >  	mtd->_read = spi_nor_read;
> > +#ifdef CONFIG_OF
> > +	np_spi = of_get_next_parent(np);
> > +
> > +	if (of_property_read_u32(np_spi, "xlnx,qspi-mode",
> > +				&xlnx_qspi_mode) < 0) {
> This really looks controller specific so should not be placed in the generic spi-
> nor.c file.

Const is removed in info struct, because to update info members based parallel configuration.
As I mentioned above,  for this parallel configuration, mtd and spi-nor should know the details like
mtd->size, info->sectors, sector_size and page_size.
So during spi_nor_scan only mtd will update all these details, that's why I have added controller specific check to update those.

Can you please suggest, any other way to let mtd and spi-nor to know about this configuration without touching the core layers?

Please let me know, if I missed providing any required info.

> 
> > +		nor->shift = 0;
> > +		nor->stripe = 0;
> > +	} else if (xlnx_qspi_mode == 2) {
> > +		nor->shift = 1;
> > +		info->sector_size <<= nor->shift;
> > +		info->page_size <<= nor->shift;
> Just don't modify *info members! :)
> 
> 
> > +		mtd->size <<= nor->shift;
> > +		nor->stripe = 1;
> > +		nor->spi->master->flags |= (SPI_MASTER_BOTH_CS |
> > +						SPI_MASTER_DATA_STRIPE);
> > +	}
> > +#else
> > +	/* Default to single */
> > +	nor->shift = 0;
> > +	nor->stripe = 0;
> > +#endif
> Best regards,
> 
> Cyrille

Thanks,
Naga Sureshkumar Relli

^ 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