Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [media] VPU: mediatek: fix dereference of pdev before checking it is null
From: Matthias Brugger @ 2016-11-18  9:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116191650.11486-1-colin.king@canonical.com>



On 16/11/16 20:16, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> pdev is dereferenced using platform_get_drvdata before a check to
> see if it is null, hence there could be a potential null pointer
> dereference issue. Instead, first check if pdev is null and only then
> deference pdev when initializing vpu.
>
> Found with static analysis by CoverityScan, CID 1357797
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---

Reviewed-by: Matthias Brugger <mbrugger@suse.com>

>  drivers/media/platform/mtk-vpu/mtk_vpu.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu.c b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> index c9bf58c..41f31b2 100644
> --- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c
> @@ -523,9 +523,9 @@ static int load_requested_vpu(struct mtk_vpu *vpu,
>
>  int vpu_load_firmware(struct platform_device *pdev)
>  {
> -	struct mtk_vpu *vpu = platform_get_drvdata(pdev);
> +	struct mtk_vpu *vpu;
>  	struct device *dev = &pdev->dev;
> -	struct vpu_run *run = &vpu->run;
> +	struct vpu_run *run;
>  	const struct firmware *vpu_fw = NULL;
>  	int ret;
>
> @@ -533,6 +533,8 @@ int vpu_load_firmware(struct platform_device *pdev)
>  		dev_err(dev, "VPU platform device is invalid\n");
>  		return -EINVAL;
>  	}
> +	vpu = platform_get_drvdata(pdev);
> +	run = &vpu->run;
>
>  	mutex_lock(&vpu->vpu_mutex);
>  	if (vpu->fw_loaded) {
>

^ permalink raw reply

* [Resend PATCH] drm/mediatek: fix null pointer dereference
From: Matthias Brugger @ 2016-11-18 10:06 UTC (permalink / raw)
  To: linux-arm-kernel

The probe function requests the interrupt before initializing
the ddp component. Which leads to a null pointer dereference at boot.
Fix this by requesting the interrput after all components got
initialized properly.

Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC
MT8173.")
Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c 
b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 019b7ca..1e78159 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -250,13 +250,6 @@ static int mtk_disp_ovl_probe(struct 
platform_device *pdev)
  	if (irq < 0)
  		return irq;

-	ret = devm_request_irq(dev, irq, mtk_disp_ovl_irq_handler,
-			       IRQF_TRIGGER_NONE, dev_name(dev), priv);
-	if (ret < 0) {
-		dev_err(dev, "Failed to request irq %d: %d\n", irq, ret);
-		return ret;
-	}
-
  	comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DISP_OVL);
  	if (comp_id < 0) {
  		dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
@@ -272,6 +265,13 @@ static int mtk_disp_ovl_probe(struct 
platform_device *pdev)

  	platform_set_drvdata(pdev, priv);

+	ret = devm_request_irq(dev, irq, mtk_disp_ovl_irq_handler,
+			       IRQF_TRIGGER_NONE, dev_name(dev), priv);
+	if (ret < 0) {
+		dev_err(dev, "Failed to request irq %d: %d\n", irq, ret);
+		return ret;
+	}
+
  	ret = component_add(dev, &mtk_disp_ovl_component_ops);
  	if (ret)
  		dev_err(dev, "Failed to add component: %d\n", ret);
-- 
2.10.1

^ permalink raw reply related

* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-18 10:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161114112625.GO10219@e106497-lin.cambridge.arm.com>

On Monday, November 14, 2016 11:26:25 AM CET liviu.dudau at arm.com wrote:
> On Mon, Nov 14, 2016 at 08:26:42AM +0000, Gabriele Paoloni wrote:
> > > Nope, that is not what it means. It means that PCI devices can see I/O
> > > addresses
> > > on the bus that start from 0. There never was any usage for non-PCI
> > > controllers
> > 
> > So I am a bit confused...
> > From http://www.firmware.org/1275/bindings/isa/isa0_4d.ps
> > It seems that ISA buses operate on cpu I/O address range [0, 0xFFF].
> > I thought that was the reason why for most architectures we have
> > PCIBIOS_MIN_IO equal to 0x1000 (so I thought that ISA controllers
> > usually use [0, PCIBIOS_MIN_IO - 1] )
> 
> First of all, cpu I/O addresses is an x86-ism. ARM architectures and others
>  have no separate address space for I/O, it is all merged into one unified
> address space. So, on arm/arm64 for example, PCIBIOS_MIN_IO = 0 could mean
> that we don't care about ISA I/O because the platform does not support having
> an ISA bus (e.g.).

I think to be more specific, PCIBIOS_MIN_IO=0 would indicate that you cannot
have a PCI-to-ISA or PCI-to-LPC bridge in any PCI domain. This is different
from having an LPC master outside of PCI, as that lives in its own domain
and has a separately addressable I/O space.

> > As said before this series forbid IO tokens to be in [0, PCIBIOS_MIN_IO)
> > to allow special ISA controllers to use that range with special
> > accessors.
> > Having a variable threshold would make life much more difficult
> > as there would be a probe dependency between the PCI controller and
> > the special ISA one (PCI to wait for the special ISA device to be
> > probed and set the right threshold value from DT or ACPI table).
> > 
> > Instead using PCIBIOS_MIN_IO is easier and should not impose much
> > constraint as [PCIBIOS_MIN_IO, IO_SPACE_LIMIT] is available to
> > the PCI controller for I/O tokens...
> 
> What I am suggesting is to leave PCIBIOS_MIN_IO alone which still reserves
> space for ISA controller and add a PCIBIOS_MIN_DIRECT_IO that will reserve
> space for your direct address I/O on top of PCIBIOS_MIN_IO.

The PCIBIOS_MIN_DIRECT_IO name still suggests having something related to
PCIBIOS_MIN_IO, but it really isn't. We are talking about multiple
concepts here that are not the same but that are somewhat related:

a) keeping PCI devices from allocating low I/O ports on the PCI bus
   that would conflict with ISA devices behind a bridge of the
   same bus.

b) reserving the low 0x0-0xfff range of the Linux-internal I/O
   space abstraction to a particular LPC or PCI domain to make
   legacy device drivers work that hardcode a particular port
   number.

c) Redirecting inb/outb to call a domain-specific accessor function
   rather than doing the normal MMIO window for an LPC master or
   more generally any arbitrary LPC or PCI domain that has a
   nonstandard I/O space. 
   [side note: actually if we generalized this, we could avoid
    assigning an MMIO range for the I/O space on the pci-mvebu
    driver, and that would help free up some other remapping
    windows]

I think there is no need to change a) here, we have PCIBIOS_MIN_IO
today and even if we don't need it, there is no obvious downside.
I would also argue that we can ignore b) for the discussion of
the HiSilicon LPC driver, we just need to assign some range
of logical addresses to each domain.

That means solving c) is the important problem here, and it
shouldn't be so hard.  We can do this either with a single
special domain as in the v5 patch series, or by generalizing it
so that any I/O space mapping gets looked up through the device
pointer of the bus master.

	Arnd

^ permalink raw reply

* [PATCH next] arm64: remove "SMP: Total of %d processors activated." message
From: Will Deacon @ 2016-11-18 10:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <6d867f8f-c7b9-42e9-1764-6f802d6b1e96@huawei.com>

On Fri, Nov 18, 2016 at 11:37:10AM +0800, Kefeng Wang wrote:
> 
> 
> On 2016/11/17 22:22, Will Deacon wrote:
> > On Thu, Nov 17, 2016 at 03:32:26PM +0800, Kefeng Wang wrote:
> >> There is a common SMP boot message in generic code on all arches,
> >> kill "SMP: Total of %d processors activated." in smp_cpus_done()
> >> on arm64.
> >>
> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> >> ---
> >> Boot message on qemu.
> >> [    0.375116] smp: Brought up 1 node, 8 CPUs
> >> [    0.383749] SMP: Total of 8 processors activated.
> >>
> >>  arch/arm64/kernel/smp.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> >> index cb87234..9db4a95 100644
> >> --- a/arch/arm64/kernel/smp.c
> >> +++ b/arch/arm64/kernel/smp.c
> >> @@ -428,7 +428,6 @@ static void __init hyp_mode_check(void)
> >>  
> >>  void __init smp_cpus_done(unsigned int max_cpus)
> >>  {
> >> -	pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
> >>  	setup_cpu_features();
> >>  	hyp_mode_check();
> >>  	apply_alternatives_all();
> > 
> > Why? Are you proposing the same change to other architectures? Are you paid
> > per patch?
> 
> The message provides no further information than the generic code, so kill it.
> Or show BogoMIPS like arm32?

Ha! No, I don't think printing the BogoMIPS is the right solution. I just
think that, if you insist on removing the harmless print, then you should
also consider doing the same thing for all the other architectures that
print the same message.

Will

^ permalink raw reply

* [PATCHv2 5/6] arm64: Use __pa_symbol for _end
From: Ard Biesheuvel @ 2016-11-18 10:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161116173217.GB3224@e104818-lin.cambridge.arm.com>

On 16 November 2016 at 17:32, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Nov 15, 2016 at 04:09:07PM -0800, Laura Abbott wrote:
>> On 11/15/2016 10:35 AM, Catalin Marinas wrote:
>> > I'm fine with __pa_symbol use entirely from under arch/arm64. But if you
>> > want to use __pa_symbol, I tried to change most (all?) places where
>> > necessary, together with making virt_to_phys() only deal with the kernel
>> > linear mapping. Not sure it looks cleaner, especially the
>> > __va(__pa_symbol()) cases (we could replace the latter with another
>> > macro and proper comment):
>>
>> I agree everything should be converted over, I was considering doing
>> that in a separate patch but this covers everything nicely. Are you
>> okay with me folding this in? (Few comments below)
>
> Yes. I would also like Ard to review it since he introduced the current
> __virt_to_phys() macro.
>

I think this is a clear improvement. I didn't dare to propose it at
the time, due to the fallout, but it is obviously much better to have
separate accessors than to have runtime tests to decide something that
is already known at compile time. My only concern is potential uses in
generic code: I think there may be something in the handling of
initramfs, or freeing the __init segment (I know it had 'init' in the
name :-)) that refers to the physical address of symbols, but I don't
remember exactly what it is.

Did you test it with a initramfs?

>> > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
>> > index eac3dbb7e313..e02f45e5ee1b 100644
>> > --- a/arch/arm64/include/asm/memory.h
>> > +++ b/arch/arm64/include/asm/memory.h
>> > @@ -169,15 +169,22 @@ extern u64                    kimage_voffset;
>> >   */
>> >  #define __virt_to_phys_nodebug(x) ({                                       \
>> >     phys_addr_t __x = (phys_addr_t)(x);                             \
>> > -   __x & BIT(VA_BITS - 1) ? (__x & ~PAGE_OFFSET) + PHYS_OFFSET :   \
>> > -                            (__x - kimage_voffset); })
>> > +   VM_BUG_ON(!(__x & BIT(VA_BITS - 1)));                           \
>> > +   ((__x & ~PAGE_OFFSET) + PHYS_OFFSET);                           \
>> > +})
>>
>> I do think this is easier to understand vs the ternary operator.
>> I'll add a comment detailing the use of __pa vs __pa_symbol somewhere
>> as well.
>
> Of course, a comment is welcome (I just did a quick hack to check that
> it works).
>
>> > --- a/arch/arm64/include/asm/mmu_context.h
>> > +++ b/arch/arm64/include/asm/mmu_context.h
>> > @@ -44,7 +44,7 @@ static inline void contextidr_thread_switch(struct task_struct *next)
>> >   */
>> >  static inline void cpu_set_reserved_ttbr0(void)
>> >  {
>> > -   unsigned long ttbr = virt_to_phys(empty_zero_page);
>> > +   unsigned long ttbr = __pa_symbol(empty_zero_page);
>> >
>> >     write_sysreg(ttbr, ttbr0_el1);
>> >     isb();
>> > @@ -113,7 +113,7 @@ static inline void cpu_install_idmap(void)
>> >     local_flush_tlb_all();
>> >     cpu_set_idmap_tcr_t0sz();
>> >
>> > -   cpu_switch_mm(idmap_pg_dir, &init_mm);
>> > +   cpu_switch_mm(__va(__pa_symbol(idmap_pg_dir)), &init_mm);
>>
>> Yes, the __va(__pa_symbol(..)) idiom needs to be macroized and commented...
>
> Indeed. At the same time we should also replace the LMADDR macro in
> hibernate.c with whatever you come up with.
>
>> > diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c
>> > index d55a7b09959b..81c03c74e5fe 100644
>> > --- a/arch/arm64/kernel/hibernate.c
>> > +++ b/arch/arm64/kernel/hibernate.c
>> > @@ -51,7 +51,7 @@
>> >  extern int in_suspend;
>> >
>> >  /* Find a symbols alias in the linear map */
>> > -#define LMADDR(x)  phys_to_virt(virt_to_phys(x))
>> > +#define LMADDR(x)  __va(__pa_symbol(x))
>>
>> ...Perhaps just borrowing this macro?
>
> Yes but I don't particularly like the name, especially since it goes
> into a .h file. Maybe __lm_sym_addr() or something else if you have a
> better idea.
>
>> > diff --git a/arch/arm64/mm/physaddr.c b/arch/arm64/mm/physaddr.c
>> > index 874c78201a2b..98dae943e496 100644
>> > --- a/arch/arm64/mm/physaddr.c
>> > +++ b/arch/arm64/mm/physaddr.c
>> > @@ -14,8 +14,8 @@ unsigned long __virt_to_phys(unsigned long x)
>> >              */
>> >             return (__x & ~PAGE_OFFSET) + PHYS_OFFSET;
>> >     } else {
>> > -           VIRTUAL_BUG_ON(x < kimage_vaddr || x >= (unsigned long)_end);
>> > -           return (__x - kimage_voffset);
>> > +           WARN_ON(1);
>>
>> Was the deletion of the BUG_ON here intentional? VIRTUAL_BUG_ON
>> is the check enabled by CONFIG_DEBUG_VIRTUAL vs just CONFIG_DEBUG_VM.
>> I intentionally kept CONFIG_DEBUG_VIRTUAL separate since the checks
>> are expensive.
>
> I wanted to always get a warning but fall back to __phys_addr_symbol()
> so that I can track down other uses of __virt_to_phys() on kernel
> symbols without killing the kernel. A better option would have been
> VIRTUAL_WARN_ON (or *_ONCE) but we don't have it. VM_WARN_ON, as you
> said, is independent of CONFIG_DEBUG_VIRTUAL.
>
> We could as well kill the system with VIRTUAL_BUG_ON in this case but I
> thought we should be more gentle until all the __virt_to_phys use-cases
> are sorted out.
>
> --
> Catalin

^ permalink raw reply

* [PATCH 1/2] ARM: dts: r8a7743: Move RST node before SYSC node
From: Geert Uytterhoeven @ 2016-11-18 10:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479464663-19054-1-git-send-email-geert+renesas@glider.be>

To preserve both alphabetical (label) and numerical ordering (unit
address).

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7743.dtsi | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7743.dtsi b/arch/arm/boot/dts/r8a7743.dtsi
index 216cb1f37f87de9a..ca0cda28a3eb5241 100644
--- a/arch/arm/boot/dts/r8a7743.dtsi
+++ b/arch/arm/boot/dts/r8a7743.dtsi
@@ -102,17 +102,17 @@
 			#power-domain-cells = <0>;
 		};
 
+		rst: reset-controller at e6160000 {
+			compatible = "renesas,r8a7743-rst";
+			reg = <0 0xe6160000 0 0x100>;
+		};
+
 		sysc: system-controller at e6180000 {
 			compatible = "renesas,r8a7743-sysc";
 			reg = <0 0xe6180000 0 0x200>;
 			#power-domain-cells = <1>;
 		};
 
-		rst: reset-controller at e6160000 {
-			compatible = "renesas,r8a7743-rst";
-			reg = <0 0xe6160000 0 0x100>;
-		};
-
 		dmac0: dma-controller at e6700000 {
 			compatible = "renesas,dmac-r8a7743",
 				     "renesas,rcar-dmac";
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/2] ARM: dts: r8a7745: Move RST node before SYSC node
From: Geert Uytterhoeven @ 2016-11-18 10:24 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1479464663-19054-1-git-send-email-geert+renesas@glider.be>

To preserve both alphabetical (label) and numerical ordering (unit
address).

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7745.dtsi | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7745.dtsi b/arch/arm/boot/dts/r8a7745.dtsi
index 0b2e2f37150fdc68..0a5d7872ce289d9b 100644
--- a/arch/arm/boot/dts/r8a7745.dtsi
+++ b/arch/arm/boot/dts/r8a7745.dtsi
@@ -102,17 +102,17 @@
 			#power-domain-cells = <0>;
 		};
 
+		rst: reset-controller at e6160000 {
+			compatible = "renesas,r8a7745-rst";
+			reg = <0 0xe6160000 0 0x100>;
+		};
+
 		sysc: system-controller at e6180000 {
 			compatible = "renesas,r8a7745-sysc";
 			reg = <0 0xe6180000 0 0x200>;
 			#power-domain-cells = <1>;
 		};
 
-		rst: reset-controller at e6160000 {
-			compatible = "renesas,r8a7745-rst";
-			reg = <0 0xe6160000 0 0x100>;
-		};
-
 		dmac0: dma-controller at e6700000 {
 			compatible = "renesas,dmac-r8a7745",
 				     "renesas,rcar-dmac";
-- 
1.9.1

^ permalink raw reply related

* spin_lock behavior with ARM64 big.Little/HMP
From: Sudeep Holla @ 2016-11-18 10:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <400ab4b8b2354c5b9283f6ed657363a0@codeaurora.org>

Hi Vikram,

On 18/11/16 02:22, Vikram Mulukutla wrote:
> Hello,
>
> This isn't really a bug report, but just a description of a frequency/IPC
> dependent behavior that I'm curious if we should worry about. The behavior
> is exposed by questionable design so I'm leaning towards don't-care.
>
> Consider these threads running in parallel on two ARM64 CPUs running
> mainline
> Linux:
>

Are you seeing this behavior with the mainline kernel on any platforms
as we have a sort of workaround for this ?

> (Ordering of lines between the two columns does not indicate a sequence of
> execution. Assume flag=0 initially.)
>
> LittleARM64_CPU @ 300MHz (e.g.A53)   |  BigARM64_CPU @ 1.5GHz (e.g. A57)
> -------------------------------------+----------------------------------
> spin_lock_irqsave(s)                 |  local_irq_save()
> /* critical section */
> flag = 1                             |  spin_lock(s)
> spin_unlock_irqrestore(s)            |  while (!flag) {
>                                      |      spin_unlock(s)
>                                      |      cpu_relax();
>                                      |      spin_lock(s)
>                                      |  }
>                                      |  spin_unlock(s)
>                                      |  local_irq_restore()
>
> I see a livelock occurring where the LittleCPU is never able to acquire the
> lock, and the BigCPU is stuck forever waiting on 'flag' to be set.
>

Yes we saw this issue 3 years back on TC2 which has A7(with lowest
frequency of 300MHz IIRC) and A15(with 1.2 GHz). We were observing that
inter-cluster events are missed since the two clusters are operating at
different frequencies (details below).

The hardware recommendation is that there should be glue logic between
the two clusters which captures events from one cluster and replays then
on the other if its operating at a different frequency.

Generally EVENTO from cluster 1 is connected to the EVENTI of the
cluster 2 and vice versa. The only extra logic required is the double
synchronizer in the receiving clock domain.

This issue arise in reality if the synchronizer is missing and different
CPUs hold EVENTO for different clock cycles.

However there was a different requirement to implement timer event
stream in Linux for some user-space locking and that indirectly help to
resolve the issue on TC2. That event stream feature is enabled by
default in Linux and should fix the issue and hence I asked you if you
still see that issue.

-- 
Regards,
Sudeep

^ permalink raw reply

* arasan,sdhci.txt "compatibility" DT binding
From: Mason @ 2016-11-18 10:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <56B21DDB.6050708@free.fr>

On 03/02/2016 16:33, Mason wrote:
> On 03/02/2016 16:21, S?ren Brinkmann wrote:
>> On Wed, 2016-02-03 at 10:58:24 +0100, Michal Simek wrote:
>>> On 3.2.2016 09:31, Mason wrote:
>>>> On 03/02/2016 08:20, Michal Simek wrote:
>>>>
>>>>> On 3.2.2016 03:33, Shawn Lin wrote:
>>>>>> + Michal, S?ren Brinkmann
>>>>>>
>>>>>> On 2016/2/2 17:49, Mason wrote:
>>>>>>> Hello everyone,
>>>>>>>
>>>>>>> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt states:
>>>>>>>
>>>>>>> Required Properties:
>>>>>>>    - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
>>>>>>>                  'arasan,sdhci-4.9a' or 'arasan,sdhci-5.1'
>>>>>>>
>>>>>>> What do 8.9a, 4.9a, and 5.1 refer to?
>>>>>>>
>>>>>>
>>>>>> Good question.
>>>>>>
>>>>>> Michal told me that 8.9a and 4.9a came from Xilinx
>>>>>> databook which define their available arasan controller to be version
>>>>>> 4.9a and 8.9a.
>>>>>
>>>>> Our version is coming from here.
>>>>> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
>>>>> page 28
>>>>>
>>>>> Datasheets from 2012 and 2010 doesn't look too recent and I expect that
>>>>> Arasan did a lot of work from that time that's why I am not surprised
>>>>> that you are not able to see that versions.
>>>>
>>>> Hello Michal,
>>>>
>>>> I'm even more confused.
>>>>
>>>> Arasan's 2010-02-19 data sheet is titled
>>>> SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller
>>>>
>>>> Page 28 of the Xilinx data sheet mentions
>>>> SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller
>>>> Version 8.9A_apr02nd_2010
>>>>
>>>> It does not make sense that Arasan would support SD3.0 in February,
>>>> then go back to SD 2.0 in April.
>>>>
>>>> I do note eMMC4.4 vs MMC3.31 => perhaps these are two *different*
>>>> IP blocks?
>>>>
>>>> Do your data sheets come with revision history?
>>>
>>> I don't have datasheet for this IP in my hand that's why I can't check it.
>>> But I can't see any problem with it. Our zynq SoC supports SD2.0 and it
>>> was requirement at that time. Bugs can happen. Arasan fixed it and
>>> create new version.
>>> At the same time can have 3.0 versions but vendor is just decide not to
>>> use it for whatever reason.
>>>
>>> That's why timing of features and versions can upgrade any time and
>>> unfortunately bugs happen.
>>
>> We have several Arasan data sheets here. The document names are:
>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Userguide" and the revisions are 9.2a,
>> 4.4a and 5.4a. I have the feeling that the document revisions have
>> been mistaken as the IP revision. I cannot find any other indicator
>> for the IP revision though. Does the IP have a way to discover its
>> revision?
> 
> To summarize, it looks to me like
> 4.9a and 8.9a are documentation revision numbers for this IP:
> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
> 
> and 5.1 seems to be the eMMC standard, so one of these IP:
> http://arasan.com/products/emmc51/sd3-emmc-5-1-host/
> http://arasan.com/products/emmc51/emmc-5-1-sd-4-1-host/
> 
> Whereas my board is using still another IP:
> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
> 
> 
> If that is correct, then I should be able to use the 8.9a
> compatible string, and perhaps my hardware will work in
> slightly degraded performance mode, from not using the
> newest protocol gizmos available.

Resurrecting this thread after a chat with Michal on IRC.

The driver now supports a few more compatible strings.
Tracing the history...

Soren/Xilinx defined "arasan,sdhci-8.9a" in e3ec3a3d11ad
They have the "SD2.0/SDIO2.0/MMC3.31 AHB Host Controller" version
(8.9a might be a document revision number, dated 2010-04-02)

Suman/APM added "arasan,sdhci-4.9a" in 308f3f8d8112
@Suman, @Rameshwar: what specific IP block does your SoC embed?
What does 4.9a refer to? The documentation revision number?

Shawn/Rockchip added "arasan,sdhci-5.1" in da795ec26e25
This seems to be for an *actual* eMMC 5.1 version
(instead of a documentation version) as mentioned in
the commit message.

Douglas added "rockchip,rk3399-sdhci-5.1"
and made several other improvements.

I have reached out to Arasan support. Hopefully they can also
help clear up the confusion, assuming they care about the
Linux driver.

Regards.

^ permalink raw reply

* [PATCH v2] ARM: Drop fixed 200 Hz timer requirement from Samsung platforms
From: Sylwester Nawrocki @ 2016-11-18 10:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <22757093.ejshJp9T7L@wuerfel>

On 11/18/2016 09:46 AM, Arnd Bergmann wrote:
> On Friday, November 18, 2016 9:16:58 AM CET Krzysztof Kozlowski wrote:
>> > All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
>> > 200 Hz.  Unfortunately in case of multiplatform image this affects also
>> > other platforms when Exynos is enabled.
>> > 
>> > This looks like an very old legacy code, dating back to initial
>> > upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
>> > driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
>> > unused plat-samsung/time.c").
>> > 
>> > Since then, this fixed 200 Hz spread everywhere, including out-of-tree
>> > Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
>> > was rather an effect of coincidence instead of conscious choice.
>> > 
>> > Exynos uses its own MCT or arch timer and can work with all HZ values.
>> > Older platforms use newer Samsung PWM timer driver which should handle
>> > down to 100 Hz.
>> > 
>> > Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
>> > A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
>> > other values.
>> > 
>> > Reported-by: Lee Jones <lee.jones@linaro.org>
>> > [Dropping 200_HZ from S3C/S5P suggested by Arnd]
>> > Reported-by: Arnd Bergmann <arnd@arndb.de>
>> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>> > Cc: Kukjin Kim <kgene@kernel.org>
>> > Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
>> > 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> Maybe add a paragraph about the specific problem:
> 
> "On s3c24xx, the PWM counter is only 16 bit wide, and with the
> typical 12MHz input clock that overflows every 5.5ms. This works
> with HZ=200 or higher but not with HZ=100 which needs a 10ms
> interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> the counter is 32 bits and does not have this problem.
> The new samsung_pwm_timer driver solves the problem by scaling
> the input clock by a factor of 50 on s3c24xx, which makes it
> less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> fewer wakeups".

I've tested on S3C2440 SoC based board and I didn't notice any
issues with HZ=100.

Clock frequencies look a bit different because AFAIU MPLL
clock is mostly used as a root clock. The 12 MHz oscillator clock
is used a root clock for the MPLL.

refclk:    12000 kHz
mpll:     405000 kHz
upll:      48000 kHz
fclk:     405000 kHz
hclk:     101250 kHz
pclk:      50625 kHz

So frequency of the timer block's source clock (PCLK) is 50.625 MHz.
This is further divided by 50 in the prescaler as you pointed out.

So the 16-bit is clocked with 1012500 Hz clock. I added some printks
to verify this.

Here is boot log for HZ=200: http://pastebin.com/JuWZdYwh
and HZ=100 http://pastebin.com/HnDnBfhc

samsung_clocksource_init:351 pclk: 50625000, timer clock_rate: 1012500
sched_clock: 16 bits at 1012kHz, resolution 987ns, wraps every 32362962ns

I just don't understand why the log says timer overflow is every 32.362 ms
and not twice this value (65536 * 1/1012500).

--
Thanks,
Sylwester

^ permalink raw reply

* arasan,sdhci.txt "compatibility" DT binding
From: Rameshwar Sahu @ 2016-11-18 10:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <582ED9E0.5090506@free.fr>

Hi Mason,


Suman/APM added "arasan,sdhci-4.9a" in 308f3f8d8112
@Suman, @Rameshwar: what specific IP block does your SoC embed?
What does 4.9a refer to? The documentation revision number?

We have Arasan SD3.0/ SDIO3.0/ eMMC4.41 AHB Host Controller IP
embedded in our SoC and 4.9a is documentation revision number which
was given by arasan.
FYI this documentation date was May, 2012.

Thanks,
Ram

On Fri, Nov 18, 2016 at 4:07 PM, Mason <slash.tmp@free.fr> wrote:
> On 03/02/2016 16:33, Mason wrote:
>> On 03/02/2016 16:21, S?ren Brinkmann wrote:
>>> On Wed, 2016-02-03 at 10:58:24 +0100, Michal Simek wrote:
>>>> On 3.2.2016 09:31, Mason wrote:
>>>>> On 03/02/2016 08:20, Michal Simek wrote:
>>>>>
>>>>>> On 3.2.2016 03:33, Shawn Lin wrote:
>>>>>>> + Michal, S?ren Brinkmann
>>>>>>>
>>>>>>> On 2016/2/2 17:49, Mason wrote:
>>>>>>>> Hello everyone,
>>>>>>>>
>>>>>>>> Documentation/devicetree/bindings/mmc/arasan,sdhci.txt states:
>>>>>>>>
>>>>>>>> Required Properties:
>>>>>>>>    - compatible: Compatibility string. Must be 'arasan,sdhci-8.9a' or
>>>>>>>>                  'arasan,sdhci-4.9a' or 'arasan,sdhci-5.1'
>>>>>>>>
>>>>>>>> What do 8.9a, 4.9a, and 5.1 refer to?
>>>>>>>>
>>>>>>>
>>>>>>> Good question.
>>>>>>>
>>>>>>> Michal told me that 8.9a and 4.9a came from Xilinx
>>>>>>> databook which define their available arasan controller to be version
>>>>>>> 4.9a and 8.9a.
>>>>>>
>>>>>> Our version is coming from here.
>>>>>> http://www.xilinx.com/support/documentation/user_guides/ug585-Zynq-7000-TRM.pdf
>>>>>> page 28
>>>>>>
>>>>>> Datasheets from 2012 and 2010 doesn't look too recent and I expect that
>>>>>> Arasan did a lot of work from that time that's why I am not surprised
>>>>>> that you are not able to see that versions.
>>>>>
>>>>> Hello Michal,
>>>>>
>>>>> I'm even more confused.
>>>>>
>>>>> Arasan's 2010-02-19 data sheet is titled
>>>>> SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller
>>>>>
>>>>> Page 28 of the Xilinx data sheet mentions
>>>>> SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller
>>>>> Version 8.9A_apr02nd_2010
>>>>>
>>>>> It does not make sense that Arasan would support SD3.0 in February,
>>>>> then go back to SD 2.0 in April.
>>>>>
>>>>> I do note eMMC4.4 vs MMC3.31 => perhaps these are two *different*
>>>>> IP blocks?
>>>>>
>>>>> Do your data sheets come with revision history?
>>>>
>>>> I don't have datasheet for this IP in my hand that's why I can't check it.
>>>> But I can't see any problem with it. Our zynq SoC supports SD2.0 and it
>>>> was requirement at that time. Bugs can happen. Arasan fixed it and
>>>> create new version.
>>>> At the same time can have 3.0 versions but vendor is just decide not to
>>>> use it for whatever reason.
>>>>
>>>> That's why timing of features and versions can upgrade any time and
>>>> unfortunately bugs happen.
>>>
>>> We have several Arasan data sheets here. The document names are:
>>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Userguide" and the revisions are 9.2a,
>>> 4.4a and 5.4a. I have the feeling that the document revisions have
>>> been mistaken as the IP revision. I cannot find any other indicator
>>> for the IP revision though. Does the IP have a way to discover its
>>> revision?
>>
>> To summarize, it looks to me like
>> 4.9a and 8.9a are documentation revision numbers for this IP:
>> "SD2.0 / SDIO2.0 / MMC3.31 AHB Host Controller"
>>
>> and 5.1 seems to be the eMMC standard, so one of these IP:
>> http://arasan.com/products/emmc51/sd3-emmc-5-1-host/
>> http://arasan.com/products/emmc51/emmc-5-1-sd-4-1-host/
>>
>> Whereas my board is using still another IP:
>> "SD3.0 / SDIO3.0 / eMMC4.4 AHB Host Controller"
>>
>>
>> If that is correct, then I should be able to use the 8.9a
>> compatible string, and perhaps my hardware will work in
>> slightly degraded performance mode, from not using the
>> newest protocol gizmos available.
>
> Resurrecting this thread after a chat with Michal on IRC.
>
> The driver now supports a few more compatible strings.
> Tracing the history...
>
> Soren/Xilinx defined "arasan,sdhci-8.9a" in e3ec3a3d11ad
> They have the "SD2.0/SDIO2.0/MMC3.31 AHB Host Controller" version
> (8.9a might be a document revision number, dated 2010-04-02)
>
> Suman/APM added "arasan,sdhci-4.9a" in 308f3f8d8112
> @Suman, @Rameshwar: what specific IP block does your SoC embed?
> What does 4.9a refer to? The documentation revision number?
>
> Shawn/Rockchip added "arasan,sdhci-5.1" in da795ec26e25
> This seems to be for an *actual* eMMC 5.1 version
> (instead of a documentation version) as mentioned in
> the commit message.
>
> Douglas added "rockchip,rk3399-sdhci-5.1"
> and made several other improvements.
>
> I have reached out to Arasan support. Hopefully they can also
> help clear up the confusion, assuming they care about the
> Linux driver.
>
> Regards.
>

^ permalink raw reply

* [RFC 5/6] ARM: dts: dra7: add vivante for bb2d module
From: Lucas Stach @ 2016-11-18 10:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118024436.13447-5-robertcnelson@gmail.com>

Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:
> Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
> CC: Julien <jboulnois@gmail.com>
> CC: Nishanth Menon <nm@ti.com>
> CC: Tomi Valkeinen <tomi.valkeinen@ti.com>
> CC: Tony Lindgren <tony@atomide.com>
> ---
>  arch/arm/boot/dts/dra7.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 43488b6..22bd0a5 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -960,7 +960,7 @@
>  		};
>  
>  		bb2d: bb2d at 59000000 {
> -			compatible = "ti,dra7-bb2d";
> +			compatible = "ti,dra7-bb2d","vivante,gc";

This is what the driver expects as a compatible, but it's not consistent
with the DT documentation patch you sent. As the driver can work out the
HW version from the identification registers I would say fix your DT
binding to only require "vivante,gc"

Regards,
Lucas

^ permalink raw reply

* [RFC 4/6] ARM: dts: dra7: add entry for bb2d module
From: Lucas Stach @ 2016-11-18 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118024436.13447-4-robertcnelson@gmail.com>

Am Donnerstag, den 17.11.2016, 20:44 -0600 schrieb Robert Nelson:
> From: Gowtham Tammana <g-tammana@ti.com>
> 
> BB2D entry is added to the dts file. Crossbar index number is used
> for interrupt mapping.
> 
> Signed-off-by: Gowtham Tammana <g-tammana@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/boot/dts/dra7.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index addb753..43488b6 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -959,6 +959,16 @@
>  			ti,hwmods = "dmm";
>  		};
>  
> +		bb2d: bb2d at 59000000 {
> +			compatible = "ti,dra7-bb2d";
> +			reg = <0x59000000 0x0700>;
> +			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
> +			ti,hwmods = "bb2d";
> +			clocks = <&dpll_core_h24x2_ck>;
> +			clock-names = "fclk";

"fclk" is not an accepted clock name for the etnaviv driver. It supports
up to 3 clocks: "bus", "core" and "shader". If there is only one clock
required in your design it would probably be the "core" clock.

Regards,
Lucas

^ permalink raw reply

* [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family flash
From: Krzeminski, Marcin (Nokia - PL/Wroclaw) @ 2016-11-18 10:59 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <DB6PR0401MB24074D231C8EBCC56E168CC989B00@DB6PR0401MB2407.eurprd04.prod.outlook.com>



> -----Original Message-----
> From: Yao Yuan [mailto:yao.yuan at nxp.com]
> Sent: Friday, November 18, 2016 5:20 AM
> To: Krzeminski, Marcin (Nokia - PL/Wroclaw)
> <marcin.krzeminski@nokia.com>; Han Xu <xhnjupt@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>; linux-
> kernel at vger.kernel.org; linux-mtd at lists.infradead.org;
> han.xu at freescale.com; Brian Norris <computersforpeace@gmail.com>;
> jagannadh.teki at gmail.com; linux-arm-kernel at lists.infradead.org
> Subject: RE: [PATCH v3 6/9] mtd: spi-nor: Support R/W for S25FS-S family
> flash
> 
> On Thu, Nov 17, 2016 at 10:14:55AM +0000, Krzeminski, Marcin (Nokia -
> PL/Wroclaw) wrote:
> > > On Thu, Nov 17, 2016 at 06:50:55AM +0000, Krzeminski, Marcin (Nokia
> > > -
> > > PL/Wroclaw) wrote:
> > > > > > > On Thu, Aug 18, 2016 at 2:38 AM, Yunhui Cui
> > > > > > > <B56489@freescale.com>
> > > > > > > wrote:
> > > > > > > > From: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > >
> > > > > > > > With the physical sectors combination, S25FS-S family
> > > > > > > > flash requires some special operations for read/write functions.
> > > > > > > >
> > > > > > > > Signed-off-by: Yunhui Cui <yunhui.cui@nxp.com>
> > > > > > > > ---
> > > > > > > >  drivers/mtd/spi-nor/spi-nor.c | 56
> > > > > > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > > > > >  1 file changed, 56 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > b/drivers/mtd/spi-nor/spi-nor.c index d0fc165..495d0bb
> > > > > > > > 100644
> > > > > > > > --- a/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > > > > > > > @@ -39,6 +39,10 @@
> > > > > > > >
> > > > > > > >  #define SPI_NOR_MAX_ID_LEN     6
> > > > > > > >  #define SPI_NOR_MAX_ADDR_WIDTH 4
> > > > > > > > +/* Added for S25FS-S family flash */
> > > > > > > > +#define SPINOR_CONFIG_REG3_OFFSET      0x800004
> > > > > > > > +#define CR3V_4KB_ERASE_UNABLE  0x8 #define
> > > > > > > > +SPINOR_S25FS_FAMILY_EXT_JEDEC  0x81
> > > > > > > >
> > > > > > > >  struct flash_info {
> > > > > > > >         char            *name;
> > > > > > > > @@ -78,6 +82,7 @@ struct flash_info {  };
> > > > > > > >
> > > > > > > >  #define JEDEC_MFR(info)        ((info)->id[0])
> > > > > > > > +#define EXT_JEDEC(info)        ((info)->id[5])
> > > > > > > >
> > > > > > > >  static const struct flash_info *spi_nor_match_id(const
> > > > > > > > char *name);
> > > > > > > >
> > > > > > > > @@ -899,6 +904,7 @@ static const struct flash_info
> spi_nor_ids[] = {
> > > > > > > >          */
> > > > > > > >         { "s25sl032p",  INFO(0x010215, 0x4d00,  64 * 1024,
> > > > > > > > 64,
> > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > >         { "s25sl064p",  INFO(0x010216, 0x4d00,  64 * 1024,
> > > > > > > > 128, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > > +       { "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 *
> > > > > > > > + 1024, 512, 0)},
> > > > > > > >         { "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
> > > > > > > >         { "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024,
> > > > > > > > 512,
> > > > > > > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > > > > > > >         { "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024,
> > > > > > > > 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) }, @@ -
> 1036,6
> > > > > +1042,50
> > > > > > > @@ static const struct flash_info *spi_nor_read_id(struct
> > > > > > > spi_nor
> > > > > > > *nor)
> > > > > > > >         return ERR_PTR(-ENODEV);  }
> > > > > > > >
> > > > > > > > +/*
> > > > > > > > + * The S25FS-S family physical sectors may be configured
> > > > > > > > +as a
> > > > > > > > + * hybrid combination of eight 4-kB parameter sectors
> > > > > > > > + * at the top or bottom of the address space with all
> > > > > > > > + * but one of the remaining sectors being uniform size.
> > > > > > > > + * The Parameter Sector Erase commands (20h or 21h) must
> > > > > > > > + * be used to erase the 4-kB parameter sectors individually.
> > > > > > > > + * The Sector (uniform sector) Erase commands (D8h or
> > > > > > > > +DCh)
> > > > > > > > + * must be used to erase any of the remaining
> > > > > > > > + * sectors, including the portion of highest or lowest
> > > > > > > > +address
> > > > > > > > + * sector that is not overlaid by the parameter sectors.
> > > > > > > > + * The uniform sector erase command has no effect on
> > > > > > > > +parameter
> > > > > > > sectors.
> > > > > > > > + */
> > > > > > > > +static int spansion_s25fs_disable_4kb_erase(struct spi_nor
> *nor) {
> > > > > > > > +       u32 cr3v_addr  = SPINOR_CONFIG_REG3_OFFSET;
> > > > > > > > +       u8 cr3v = 0x0;
> > > > > > > > +       int ret = 0x0;
> > > > > > > > +
> > > > > > > > +       nor->cmd_buf[2] = cr3v_addr >> 16;
> > > > > > > > +       nor->cmd_buf[1] = cr3v_addr >> 8;
> > > > > > > > +       nor->cmd_buf[0] = cr3v_addr >> 0;
> > > > > > > > +
> > > > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > > &cr3v, 1);
> > > > > > > > +       if (ret)
> > > > > > > > +               return ret;
> > > > > > > > +       if (cr3v & CR3V_4KB_ERASE_UNABLE)
> > > > > > > > +               return 0;
> > > > > > > > +       ret = nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
> > > > > > > > +       if (ret)
> > > > > > > > +               return ret;
> > > > > > > > +       cr3v = CR3V_4KB_ERASE_UNABLE;
> > > > > > > > +       nor->program_opcode = SPINOR_OP_SPANSION_WRAR;
> > > > > > > > +       nor->write(nor, cr3v_addr, 1, &cr3v);
> > > > > > > > +
> > > > > > > > +       ret = nor->read_reg(nor, SPINOR_OP_SPANSION_RDAR,
> > > &cr3v, 1);
> > > > > > > > +       if (ret)
> > > > > > > > +               return ret;
> > > > > > > > +       if (!(cr3v & CR3V_4KB_ERASE_UNABLE))
> > > > > > > > +               return -EPERM;
> > > > > > > > +
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +
> > > > > > > >  static int spi_nor_read(struct mtd_info *mtd, loff_t
> > > > > > > > from, size_t
> > > len,
> > > > > > > >                         size_t *retlen, u_char *buf)  { @@
> > > > > > > > -1361,6
> > > > > > > > +1411,12 @@ int spi_nor_scan(struct spi_nor *nor, const
> > > > > > > > +char *name,
> > > > > > > enum read_mode mode)
> > > > > > > >                 spi_nor_wait_till_ready(nor);
> > > > > > > >         }
> > > > > > > >
> > > > > > > > +       if (EXT_JEDEC(info) == SPINOR_S25FS_FAMILY_EXT_JEDEC)
> {
> > > > > > > > +               ret = spansion_s25fs_disable_4kb_erase(nor);
> > > > > > > > +               if (ret)
> > > > > > > > +                       return ret;
> > > > > > > > +       }
> > > > > > > > +
> > > > > > > >         if (!mtd->name)
> > > > > > > >                 mtd->name = dev_name(dev);
> > > > > > > >         mtd->priv = nor;
> > > > > > > > --
> > > > > > > > 2.1.0.27.g96db324
> > > > > > > >
> > > > > > > >
> > > > > > > Hi Brian, I will ack this change but still feel it's kind of hacking code.
> > > > > > >
> > > > > > > Acked-by: Han xu <han.xu@nxp.com>
> > > > > >
> > > > > > I am new on the list so I am not sure if this topic has been discussed.
> > > > > > Generally our product functionality relay on those 4KiB sectors.
> > > > > > I know that this hack is already in u-boot, but if you
> > > > > > mainstream this you will force users of those 4KiB sectors to
> > > > > > do hack the
> > hack...
> > > > > > I believe the proper solution here is to use erase regions
> > > > > > functionality, I send and RFS about that some time ago.
> > > > >
> > > > > Do you mind to send me a link for reference?
> > > > >
> > > > Han,
> > > >
> > > > Sorry, It seem I have not posted erase region changes (only those
> > > > regarding DUAL/QUAD I/O).
> > > > Generally, in this flash you need to create 3 erase regions
> > > > (because in FS-S family support only  4KiB erase on parameters sector -
> eg.
> > > > 1.2.2.4 in
> > > S25FS512S).
> > > > In my case regions are:
> > > > 1. 0-32KiB (8*4KiB) - 4K_ERASE (0x20/0x21) 2. 32 - 256 - SE_CMD
> > > (0xd8/0xdc) 3.
> > > > Rest of the flash SE_CMD (0xd8/0xdc)
> > > >
> > > > To erase whole flash you can also use CHIP_ERASE_CMD (0x60/0xC7)
> > > > command, you just need to add one more mtd partition that will
> > > > cover
> > > whole flash.
> > > >
> > >
> > > Hi Krzeminski,
> > >
> > > Do you think is there any great advantages for enable 4KB?
> > > Because for NXP(Freescale) QSPI controller, there is only support
> > > max to 16 groups command.
> > >
> > > So It's hard to give 3 groups command just for erase (0x21,0xdc and 0xc7).
> > > So we have to disable the 4kb erase and only use 256kbytes in this patch.
> > >
> > Yes, if you disable parameters sector in spi-nor framework you will
> > disable it for all spi-nor clients not only for NXP QSPI controller.
> > There are users (at least me) that relay on parameters sector functionality.
> This patch will brake it.
> >
> > Thanks,
> 
> Hi Krzeminski,
> 
> Get it.
> So do you think how about that I add a flag in dts to select it?
> The user want's disable 4kb, he can add the flag.
> 
> In spi-nor.c:
> if (of_property_read_bool(np, "spi-nor, disable-4kb")) {
> 	spansion_s25fs_disable_4kb_erase();
> }
>                 else
> ...
> 
> In dts:
> 
> &qspi {
>         num-cs = <2>;
>         bus-num = <0>;
>         status = "okay";
> 
>         qflash0: s25fs512s at 0 {
>                 compatible = "spansion, s25fs512s";
> 	 spi-nor, disable-4kb
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>                 spi-max-frequency = <20000000>;
>                 reg = <0>;
>         };
> 
> I think it should be a better way.
> 
> How about your think?

This looks much better - at least for me.
There are some parameters in JESD216 standard regarding parameters sector,
but unfortunately I have not investigated that. You can take a look at Cyrille series,
that adds support for JESD216  standard.

Thanks,
Marcin

> 
> Thanks.
> 

^ permalink raw reply

* [arm-soc:qcom/arm64 3/13] Error: arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi:12.20-21 syntax error
From: kbuild test robot @ 2016-11-18 11:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Srinivas,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git qcom/arm64
head:   feeaf56ac78d283efe65ea60ec999d4bf3cf395e
commit: 50784e61032d89cbbc46ed73a5fb15f27940b947 [3/13] dts: arm64: db820c: add pmic pins specific dts file
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 50784e61032d89cbbc46ed73a5fb15f27940b947
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> Error: arch/arm64/boot/dts/qcom/apq8096-db820c-pmic-pins.dtsi:12.20-21 syntax error
   FATAL ERROR: Unable to parse input tree

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

^ permalink raw reply

* System/uncore PMUs and unit aggregation
From: Jan Glauber @ 2016-11-18 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161117181708.GT22855@arm.com>

On Thu, Nov 17, 2016 at 06:17:08PM +0000, Will Deacon wrote:
> Hi all,
> 
> We currently have support for three arm64 system PMUs in flight:
> 
>  [Cavium ThunderX] http://lkml.kernel.org/r/cover.1477741719.git.jglauber at cavium.com
>  [Hisilicon Hip0x] http://lkml.kernel.org/r/1478151727-20250-1-git-send-email-anurup.m at huawei.com
>  [Qualcomm L2] http://lkml.kernel.org/r/1477687813-11412-1-git-send-email-nleeder at codeaurora.org
> 
> Each of which have to deal with multiple underlying hardware units in one
> way or another. Mark and I recently expressed a desire to expose these
> units to userspace as individual PMU instances, since this can allow:
> 
>   * Fine-grained control of events from userspace, when you want to see
>     individual numbers as opposed to a summed total
> 
>   * Potentially ease migration to new SoC revisions, where the units
>     are laid out slightly differently
> 
>   * Easier handling of cases where the units aren't quite identical
> 
> however, this received pushback from all of the patch authors, so there's
> clearly a problem with this approach. I'm hoping we can try to resolve
> this here.

Good to know. Thanks for adressing this on a higher level.

> Speaking to Mark earlier today, we came up with the following rough rules
> for drivers that present multiple hardware units as a single PMU:
> 
>   1. If the units share some part of the programming interface (e.g. control
>      registers or interrupts), then they must be handled by the same PMU.
>      Otherwise, they should be treated independently as separate PMU
>      instances.

Can you elaborate why they should be treated independent in the later
case? What is the problem with going through a list and writing the
control register per unit?

>   2. If the units are handled by the same PMU, then care must be taken to
>      handle event groups correctly. That is, if the units cannot be started
>      and stopped atomically, cross-unit groups must be rejected by the
>      driver. Furthermore, any cross-unit scheduling constraints must be
>      honoured so that all the units targetted by a group can schedule the
>      group concurrently.
> 
>   3. Summing the counters across units is only permitted if the units
>      can all be started and stopped atomically. Otherwise, the counters
>      should be exposed individually. It's up to the driver author to
>      decide what makes sense to sum.

Do you mean started/stopped atomically across units?

>   4. Unit topology can optionally be described in sysfs (we should pick
>      some standard directory naming here), and then events targetting
>      specific units can have the unit identifier extracted from the topology
>      encoded in some configN fields.
> 
> The million dollar question is: how does that fit in with the drivers I
> mentioned at the top? Is this overly restrictive, or have we missed stuff?
> 
> We certainly want to allow flexibility in the way in which the drivers
> talk to the hardware, but given that these decisions directly affect the
> user ABI, some consistent ground rules are required.
> 
> For Cavium ThunderX, it's not clear whether or not the individual units
> could be expressed as separate PMUs, or whether they're caught by one of
> the rules above. The Qualcomm L2 looks like it's doing the right thing
> and we can't quite work out what the Hisilicon Hip0x topology looks like,
> since the interaction with djtag is confusing.

On Cavium ThunderX the current patches add 4 PMU types, which unfortunately
are all handled different. The L2C-TAD and OCX-TLK have control
registers per unit. The LMC and L2C-CBC don't have control registers,
(free-running counters). So rule 1 might be too restrictive.

I've not looked into groups, would these allow to merge counters from
different PMUs in the kernel?

--Jan

> If the driver authors (on To:) could shed some light on this, then that
> would be much appreciated!
> 
> Thanks,
> 
> Will

^ permalink raw reply

* [PATCH v2] ARM: Drop fixed 200 Hz timer requirement from Samsung platforms
From: Krzysztof Kozlowski @ 2016-11-18 11:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <8dfbb04f-d377-b51a-3010-481a5f977507@samsung.com>

On Fri, Nov 18, 2016 at 11:43:14AM +0100, Sylwester Nawrocki wrote:
> On 11/18/2016 09:46 AM, Arnd Bergmann wrote:
> > On Friday, November 18, 2016 9:16:58 AM CET Krzysztof Kozlowski wrote:
> >> > All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
> >> > 200 Hz.  Unfortunately in case of multiplatform image this affects also
> >> > other platforms when Exynos is enabled.
> >> > 
> >> > This looks like an very old legacy code, dating back to initial
> >> > upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
> >> > driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
> >> > unused plat-samsung/time.c").
> >> > 
> >> > Since then, this fixed 200 Hz spread everywhere, including out-of-tree
> >> > Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
> >> > was rather an effect of coincidence instead of conscious choice.
> >> > 
> >> > Exynos uses its own MCT or arch timer and can work with all HZ values.
> >> > Older platforms use newer Samsung PWM timer driver which should handle
> >> > down to 100 Hz.
> >> > 
> >> > Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
> >> > A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
> >> > other values.
> >> > 
> >> > Reported-by: Lee Jones <lee.jones@linaro.org>
> >> > [Dropping 200_HZ from S3C/S5P suggested by Arnd]
> >> > Reported-by: Arnd Bergmann <arnd@arndb.de>
> >> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> >> > Cc: Kukjin Kim <kgene@kernel.org>
> >> > Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
> >> > 
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > Maybe add a paragraph about the specific problem:
> > 
> > "On s3c24xx, the PWM counter is only 16 bit wide, and with the
> > typical 12MHz input clock that overflows every 5.5ms. This works
> > with HZ=200 or higher but not with HZ=100 which needs a 10ms
> > interval between ticks. On Later chips (S3C64xx, S5P and EXYNOS),
> > the counter is 32 bits and does not have this problem.
> > The new samsung_pwm_timer driver solves the problem by scaling
> > the input clock by a factor of 50 on s3c24xx, which makes it
> > less accurate but allows HZ=100 as well as CONFIG_NO_HZ with
> > fewer wakeups".
> 
> I've tested on S3C2440 SoC based board and I didn't notice any
> issues with HZ=100.
> 
> Clock frequencies look a bit different because AFAIU MPLL
> clock is mostly used as a root clock. The 12 MHz oscillator clock
> is used a root clock for the MPLL.
> 
> refclk:    12000 kHz
> mpll:     405000 kHz
> upll:      48000 kHz
> fclk:     405000 kHz
> hclk:     101250 kHz
> pclk:      50625 kHz
> 
> So frequency of the timer block's source clock (PCLK) is 50.625 MHz.
> This is further divided by 50 in the prescaler as you pointed out.
> 
> So the 16-bit is clocked with 1012500 Hz clock. I added some printks
> to verify this.
> 
> Here is boot log for HZ=200: http://pastebin.com/JuWZdYwh
> and HZ=100 http://pastebin.com/HnDnBfhc
> 
> samsung_clocksource_init:351 pclk: 50625000, timer clock_rate: 1012500
> sched_clock: 16 bits at 1012kHz, resolution 987ns, wraps every 32362962ns

Thanks for tests! I really appreciate it.

> I just don't understand why the log says timer overflow is every 32.362 ms
> and not twice this value (65536 * 1/1012500).

The answer could be at kernel/time/clocksource.c:
 * NOTE: This function includes a safety margin of 50%, in other words, we
 * return half the number of nanoseconds the hardware counter can technically
 * cover.

Best regards,
Krzysztof

^ permalink raw reply

* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: zhichang.yuan @ 2016-11-18 11:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <2436881.9RqUVYxmDf@wuerfel>

Hi, Arnd,


On 2016/11/18 17:20, Arnd Bergmann wrote:
> On Friday, November 11, 2016 6:07:07 PM CET zhichang.yuan wrote:
>>
>> I have similar idea as your PPC MMIO.
>>
>> We notice the prototype of {in/out()} is something like that:
>>
>> static inline u8 inb(unsigned long addr)
>> static inline void outb(u8 value, unsigned long addr)
>>
>> The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
>> So, could you divide this 'addr' into several bit segments? The top 8 bits is
>> defined as bus index. For normal direct IO, the bus index is 0. For those bus
>> device which need indirectIO or some special I/O accessors, when these devices
>> are initializing, can request to allocate an unique ID to them, and register
>> their own accessors to the entry which is corresponding to the ID.
> 
> Ah, have you looked at the IA64 code? It does exactly this.
> For ARM64 we decided to use the same basic approach as powerpc with
> a single range of virtual memory for mapping it as that somewhat
> simplified all cases we knew about at the time.

Yes. I spent some time to trace how to work on PPC. But the code is a bit long,
I am not clear on how the indirectIO there was supported.

I noticed there are CONFIG_PPC_INDIRECT_PIO and CONFIG_PPC_INDIRECT_MMIO on PPC.
It seems that only CONFIG_PPC_INDIRECT_MMIO applied some MSB to store the bus
tokens which are used to get iowa_busses[] for specific operation helpers.
I can not find how CONFIG_PPC_INDIRECT_PIO support multiple ISA domains. It
seems only Opal-lpc.c adopt this INDIRECT_PIO method.

Although CONFIG_PPC_INDIRECT_MMIO is for MMIO, seems not suitable for ISA/LPC
I/O. But this idea is helpful.

what else did I miss??

> 
>> In this way, we can support multiple domains, I think.
>> But I am not sure whether it is feasible, for example, are there some
>> architectures/platforms had populated the top 8 bits? Do we need to request IO
>> region from ioport_resource for those devices?  etc...
> 
> On a 64-bit architecture, the top 32 bits of the port number are
> definitely free to use for this, and 8 bits are probably sufficient.
> 
> Even on 32 bit architectures, I can't see why we'd ever need more than
> 16 bits worth of addressing within a domain, so using 8 bit domain
> and 16 bit address leaves 8 or 40 unused bits.

Yes. 8 bits are enough.
But the maximal PIO on some architectures are defined as ~0 or -1. There is no
any bare space left. Probably we can not ensure the upper 8 bits available.


Thanks,
Zhichang


> 
> 	Arnd
> 
> .
> 

^ permalink raw reply

* [PATCH v3] ARM: Drop fixed 200 Hz timer requirement from Samsung platforms
From: Krzysztof Kozlowski @ 2016-11-18 11:15 UTC (permalink / raw)
  To: linux-arm-kernel

All Samsung platforms, including the Exynos, are selecting HZ_FIXED with
200 Hz.  Unfortunately in case of multiplatform image this affects also
other platforms when Exynos is enabled.

This looks like an very old legacy code, dating back to initial
upstreaming of S3C24xx.  Probably it was required for s3c24xx timer
driver, which was removed in commit ad38bdd15d5b ("ARM: SAMSUNG: Remove
unused plat-samsung/time.c").

Since then, this fixed 200 Hz spread everywhere, including out-of-tree
Samsung kernels (SoC vendor's and Tizen's).  I believe this choice
was rather an effect of coincidence instead of conscious choice.

On S3C24xx, the PWM counter is only 16 bit wide, and with the
typical 12MHz input clock that overflows every 5.5ms.  This works
with HZ=200 or higher but not with HZ=100 which needs a 10ms
interval between ticks.  On Later chips (S3C64xx, S5P and EXYNOS),
the counter is 32 bits and does not have this problem.

The new samsung_pwm_timer driver solves the problem by scaling the input
clock by a factor of 50 on S3C24xx, which makes it less accurate but
allows HZ=100 as well as CONFIG_NO_HZ with fewer wakeups.

Few perf mem and sched tests on Odroid XU3 board (Exynos5422, 4x Cortex
A7, 4x Cortex A15) show no regressions when switching from 200 Hz to
other values.

Reported-by: Lee Jones <lee.jones@linaro.org>
[Dropping of 200_HZ from S3C/S5P was suggested by Arnd]
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Kukjin Kim <kgene@kernel.org>
[Tested on Exynos5800]
Tested-by: Javier Martinez Canillas <javier@osg.samsung.com>
Acked-by: Kukjin Kim <kgene@kernel.org>
[Tested on S3C2440]
Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

---

Changes since v2:
1. Extend message.
2. Add Kukjin's ack.
3. Add Sylwester's tested-by.

Changes since v1:
1. Add Javier's tested-by.
2. Drop HZ_FIXED also from ARCH_S5PV210 and ARCH_S3C24XX after Arnd
   suggestions and analysis.
---
 arch/arm/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529fdffab..ced2e08a9d08 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1496,8 +1496,7 @@ source kernel/Kconfig.preempt
 
 config HZ_FIXED
 	int
-	default 200 if ARCH_EBSA110 || ARCH_S3C24XX || \
-		ARCH_S5PV210 || ARCH_EXYNOS4
+	default 200 if ARCH_EBSA110
 	default 128 if SOC_AT91RM9200
 	default 0
 
-- 
2.7.4

^ permalink raw reply related

* [arm-soc:qcom/arm64 11/13] arch/arm64/boot/dts/qcom/msm8992.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
From: kbuild test robot @ 2016-11-18 11:30 UTC (permalink / raw)
  To: linux-arm-kernel

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git qcom/arm64
head:   feeaf56ac78d283efe65ea60ec999d4bf3cf395e
commit: 6a6d1978f9c0d818b4370903e1f3eecf1681c932 [11/13] arm64: dts: msm8992 SoC and LG Bullhead (Nexus 5X) support
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout 6a6d1978f9c0d818b4370903e1f3eecf1681c932
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   In file included from arch/arm64/boot/dts/qcom/msm8992-bullhead-rev-101.dts:16:0:
>> arch/arm64/boot/dts/qcom/msm8992.dtsi:14:48: fatal error: dt-bindings/clock/qcom,gcc-msm8994.h: No such file or directory
    #include <dt-bindings/clock/qcom,gcc-msm8994.h>
                                                   ^
   compilation terminated.

vim +14 arch/arm64/boot/dts/qcom/msm8992.dtsi

     8	 * but WITHOUT ANY WARRANTY; without even the implied warranty of
     9	 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    10	 * GNU General Public License for more details.
    11	 */
    12	
    13	#include <dt-bindings/interrupt-controller/arm-gic.h>
  > 14	#include <dt-bindings/clock/qcom,gcc-msm8994.h>
    15	
    16	/ {
    17		model = "Qualcomm Technologies, Inc. MSM 8992";

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

^ permalink raw reply

* [PATCH V5 1/3] ARM64 LPC: Indirect ISA port IO introduced
From: Arnd Bergmann @ 2016-11-18 11:38 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <582EE223.4090605@hisilicon.com>

On Friday, November 18, 2016 7:12:35 PM CET zhichang.yuan wrote:
> Hi, Arnd,
> 
> 
> On 2016/11/18 17:20, Arnd Bergmann wrote:
> > On Friday, November 11, 2016 6:07:07 PM CET zhichang.yuan wrote:
> >>
> >> I have similar idea as your PPC MMIO.
> >>
> >> We notice the prototype of {in/out()} is something like that:
> >>
> >> static inline u8 inb(unsigned long addr)
> >> static inline void outb(u8 value, unsigned long addr)
> >>
> >> The type of parameter 'addr' is unsigned long. For I/O space, it is big enough.
> >> So, could you divide this 'addr' into several bit segments? The top 8 bits is
> >> defined as bus index. For normal direct IO, the bus index is 0. For those bus
> >> device which need indirectIO or some special I/O accessors, when these devices
> >> are initializing, can request to allocate an unique ID to them, and register
> >> their own accessors to the entry which is corresponding to the ID.
> > 
> > Ah, have you looked at the IA64 code? It does exactly this.
> > For ARM64 we decided to use the same basic approach as powerpc with
> > a single range of virtual memory for mapping it as that somewhat
> > simplified all cases we knew about at the time.
> 
> Yes. I spent some time to trace how to work on PPC. But the code is a bit long,
> I am not clear on how the indirectIO there was supported.
> 
> I noticed there are CONFIG_PPC_INDIRECT_PIO and CONFIG_PPC_INDIRECT_MMIO on PPC.
> It seems that only CONFIG_PPC_INDIRECT_MMIO applied some MSB to store the bus
> tokens which are used to get iowa_busses[] for specific operation helpers.
> I can not find how CONFIG_PPC_INDIRECT_PIO support multiple ISA domains. It
> seems only Opal-lpc.c adopt this INDIRECT_PIO method.
> 
> Although CONFIG_PPC_INDIRECT_MMIO is for MMIO, seems not suitable for ISA/LPC
> I/O. But this idea is helpful.
> 
> what else did I miss??

I mentioned two different things here: ia64 IIRC uses some bits of the
port number to look up the domain, while powerpc traditionally had no
support for any such lookup, it did the same thing as ARM64 with
virtual remapping of MMIO ranges into an address range starting at
a fixed virtual address.

CONFIG_PPC_INDIRECT_PIO is a fairly recent addition, I was not thinking
of that.

> >> In this way, we can support multiple domains, I think.
> >> But I am not sure whether it is feasible, for example, are there some
> >> architectures/platforms had populated the top 8 bits? Do we need to request IO
> >> region from ioport_resource for those devices?  etc...
> > 
> > On a 64-bit architecture, the top 32 bits of the port number are
> > definitely free to use for this, and 8 bits are probably sufficient.
> > 
> > Even on 32 bit architectures, I can't see why we'd ever need more than
> > 16 bits worth of addressing within a domain, so using 8 bit domain
> > and 16 bit address leaves 8 or 40 unused bits.
> 
> Yes. 8 bits are enough.
> But the maximal PIO on some architectures are defined as ~0 or -1. There is no
> any bare space left. Probably we can not ensure the upper 8 bits available.

Right, we clearly can't use it across all architectures. The trick with
architectures using ULONG_MAX as the limit for port numbers is that they
treat it as a 1:1 mapping between port numbers and virtual addresses,
which is yet another way to handle the MMIO-based devices, but that has
a number of downsides we don't need to get into now.

What I think the code should do is a generic workaround handling that
architectures can opt-in to. We'd start doing this on ARM64 only,
and can then decide whether to change ARM or PowerPC over to use
that as well.

	Arnd

^ permalink raw reply

* [PATCH V5 3/3] ARM64 LPC: LPC driver implementation on Hip06
From: Arnd Bergmann @ 2016-11-18 11:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <582469C8.9090609@hisilicon.com>

[found this old mail in my drafts folder, might as well send it now]

On Thursday, November 10, 2016 8:36:24 PM CET zhichang.yuan wrote:
> Sorry! I can't catch your idea yet:(
> 
> When to register the I/O range? Is it done just after the successfully
> of_translate_address() during the children scanning?

No, you do it when first finding the bus itself, just like we do for
PCI host bridges.

> If yes, when a child is scanning, there is no range data in arm64_extio_ops. The
> addr_is_indirect_io() calling in of_get_isa_indirect_io() don't need. All we can
> check is just whether the address to be translated is IO and is under a parent
> device which has no 'ranges' property.

The children should only be scanned after the I/O range has been
registered for the parent.

> > Your current version has
> > 
> >         if (arm64_extio_ops->pfout)                             \
> >                 arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> >                        addr, value, sizeof(type));             \
> > 
> > Instead, just subtract the start of the range from the logical
> > port number to transform it back into a bus-local port number:
> > 
> >         if (arm64_extio_ops->pfout)                             \
> >                 arm64_extio_ops->pfout(arm64_extio_ops->devpara,\
> >                        addr - arm64_extio_ops->start, value, sizeof(type)); \
> > 
> I think there is some information needed sync.
> In the old patch-set, we don't bypass the pci_address_to_pio() after
> successfully of_translate_address(). In this way, we don't need to reserve any
> PIO space for our LPC since the logical port are from the same mapping
> algorithm. Based on this way, the port number in the device resource is logical
> one, then we need to subtract the start of the resource to get back the
> bus-local port.
> 
> From V3, we don't apply the mapping based on pci_address_to_pio(), the
> of_translate_address() return the bus-local port directly and store into
> relevant device resource. So, in the current arm64_extio_ops->pfout(), the
> reverse translation don't need anymore. The input "addr" is bus-local port now.

Ok, so this would have to be changed again: If we want to support multiple
bus domains, of_translate_address() must translate between the bus specific
address and the general Linux I/O port number. Even without doing that,
it seems nicer to not overlap the range of the first PCI host bridge.

	Arnd

^ permalink raw reply

* [GIT PULL 2/3] ARM64: dts: exynos: DT for v4.10
From: Krzysztof Kozlowski @ 2016-11-18 11:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161118015954.GP8578@localhost>

On Thu, Nov 17, 2016 at 05:59:54PM -0800, Olof Johansson wrote:
> On Tue, Nov 08, 2016 at 08:26:29PM +0200, Krzysztof Kozlowski wrote:
> > Hi,
> > 
> > Exynos5433 + two boards using it. Mobile boards! :)
> > 
> > I am really happy to push it. I know that it has been a lot of effort
> > in Samsung to mainline this.
> > 
> > Best regards,
> > Krzysztof
> > 
> > 
> > The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:
> > 
> >   Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git tags/samsung-dt64-4.10
> > 
> > for you to fetch changes up to 8ac46fc57df82efbc19194dddd335b6c7a960c31:
> > 
> >   arm64: dts: exynos: Add dts file for Exynos5433-based TM2E board (2016-11-03 22:19:57 +0200)
> > 
> > ----------------------------------------------------------------
> > Finally, I am really pleased to announce adding support for Exynos5433 ARMv8
> > SoC along with two boards.  A lot of Samsung people contributed into this
> > but the final work and commits were done by Chanwoo Choi.
> > 
> > This means that for v4.10 we got:
> > 1. Exynos5433 DTSI.
> > 2. Two boards: TM2 and TM2E.  These are (almost fully) working mobile phones.
> 
> Awesome! Looks like TM2 is a Tizen reference board? Great to see the support,
> even if it's taken a while.

Yes, I think these were planned to be reference devices. Userspace
packages and snapshots are published for some quite long time
(http://download.tizen.org/snapshots/tizen/3.0-mobile/latest/images/arm64-wayland/).

Best regards,
Krzysztof

^ permalink raw reply

* [GIT PULL v2] firmware: SCPI updates for v4.10
From: Sudeep Holla @ 2016-11-18 11:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161028112920.GB12241@e107155-lin>

Hi ARM-SoC Team,

I have decoupled the platform specific binding from generic SCPI. Also
I have renamed "arm,legacy-scpi" to "arm,scpi-pre-1.0". Since I haven't
heard back any objections from Olof/Rob for my response, I am sending
the pull request now.

--
Regards,
Sudeep

The following changes since commit 1001354ca34179f3db924eb66672442a173147dc:

  Linux 4.9-rc1 (2016-10-15 12:17:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git tags/scpi-updates-4.10

for you to fetch changes up to 8358c6b5fc8c160d0af8654f313b8a7745f8e304:

  firmware: arm_scpi: add support for pre-v1.0 SCPI compatible (2016-11-17 16:31:20 +0000)

----------------------------------------------------------------
SCPI updates for v4.10

1. Adds support for pre-v1.0 SCPI protocol versions

2. Adds support for SCPI used on Amlogic GXBB SoC platforms using the
   newly added pre-v1.0 SCPI protocol

3. Decouples some platform specific details from generic SCPI binding

----------------------------------------------------------------
Neil Armstrong (4):
      firmware: arm_scpi: increase MAX_DVFS_OPPS to 16 entries
      firmware: arm_scpi: add alternative legacy structures, functions and macros
      firmware: arm_scpi: allow firmware with get_capabilities not implemented
      Documentation: bindings: Add support for Amlogic GXBB SCPI protocol

Sudeep Holla (4):
      firmware: arm_scpi: add command indirection to support legacy commands
      Documentation: bindings: decouple juno specific details from generic binding
      Documentation: bindings: add compatible specific to pre v1.0 SCPI protocols
      firmware: arm_scpi: add support for pre-v1.0 SCPI compatible

 .../devicetree/bindings/arm/amlogic,scpi.txt       |  20 ++
 Documentation/devicetree/bindings/arm/arm,scpi.txt |  25 +-
 .../devicetree/bindings/arm/juno,scpi.txt          |  26 ++
 drivers/firmware/arm_scpi.c                        | 276 ++++++++++++++++++---
 4 files changed, 301 insertions(+), 46 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/amlogic,scpi.txt
 create mode 100644 Documentation/devicetree/bindings/arm/juno,scpi.txt

^ permalink raw reply

* [PATCH 15/20] ARM/hw_breakpoint: Convert to hotplug state machine
From: Will Deacon @ 2016-11-18 12:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20161117183541.8588-16-bigeasy@linutronix.de>

On Thu, Nov 17, 2016 at 07:35:36PM +0100, Sebastian Andrzej Siewior wrote:
> Install the callbacks via the state machine and let the core invoke
> the callbacks on the already online CPUs.
> 
> smp_call_function_single() has been removed because the function is already
> invoked on the target CPU.
> 
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/arm/kernel/hw_breakpoint.c | 44 ++++++++++++++++++-----------------------
>  1 file changed, 19 insertions(+), 25 deletions(-)

[...]

>  static int __init arch_hw_breakpoint_init(void)
>  {
> +	int ret;
> +
>  	debug_arch = get_debug_arch();
>  
>  	if (!debug_arch_supported()) {
> @@ -1072,8 +1069,6 @@ static int __init arch_hw_breakpoint_init(void)
>  	core_num_brps = get_num_brps();
>  	core_num_wrps = get_num_wrps();
>  
> -	cpu_notifier_register_begin();
> -
>  	/*
>  	 * We need to tread carefully here because DBGSWENABLE may be
>  	 * driven low on this core and there isn't an architected way to
> @@ -1082,15 +1077,18 @@ static int __init arch_hw_breakpoint_init(void)
>  	register_undef_hook(&debug_reg_hook);
>  
>  	/*
> -	 * Reset the breakpoint resources. We assume that a halting
> -	 * debugger will leave the world in a nice state for us.
> +	 * Register CPU notifier which resets the breakpoint resources. We
> +	 * assume that a halting debugger will leave the world in a nice state
> +	 * for us.
>  	 */
> -	on_each_cpu(reset_ctrl_regs, NULL, 1);
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm/hw_breakpoint:online",
> +				dbg_reset_online, NULL);

I'm slightly unsure about this. The dbg_reset_online callback can execute
undefined instructions (unfortunately, there's no way to probe the presence
of some of the debug registers), so it absolutely has to run within the 
register_undef_hook/unregister_undef_hook calls that are in this function.

With this patch, I worry that the callback can be postponed to ONLINE time
for other CPUs, and then the kernel will panic.

Will

^ 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