Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: Nicolas Saenz Julienne @ 2020-06-04 17:01 UTC (permalink / raw)
  To: Bhupesh Sharma, John Donnelly
  Cc: chenzhou, Simon Horman, Devicetree List, Arnd Bergmann,
	Baoquan He, Linux Doc Mailing List, Catalin Marinas, guohanjun,
	kexec mailing list, Linux Kernel Mailing List, Will Deacon,
	Rob Herring, James Morse, Prabhakar Kushwaha, Thomas Gleixner,
	Prabhakar Kushwaha, RuiRui Yang, Ingo Molnar, linux-arm-kernel
In-Reply-To: <CACi5LpN-+NRnaDoWWWidbzma8BNzmofA5FQBV=cPF1Mc84FpFg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 16696 bytes --]

On Thu, 2020-06-04 at 01:17 +0530, Bhupesh Sharma wrote:
> Hi All,
> 
> On Wed, Jun 3, 2020 at 9:03 PM John Donnelly <john.p.donnelly@oracle.com>
> wrote:
> > 
> > 
> > > On Jun 3, 2020, at 8:20 AM, chenzhou <chenzhou10@huawei.com> wrote:
> > > 
> > > Hi,
> > > 
> > > 
> > > On 2020/6/3 19:47, Prabhakar Kushwaha wrote:
> > > > Hi Chen,
> > > > 
> > > > On Tue, Jun 2, 2020 at 8:12 PM John Donnelly <john.p.donnelly@oracle.com
> > > > > wrote:
> > > > > 
> > > > > > On Jun 2, 2020, at 12:38 AM, Prabhakar Kushwaha <
> > > > > > prabhakar.pkin@gmail.com> wrote:
> > > > > > 
> > > > > > On Tue, Jun 2, 2020 at 3:29 AM John Donnelly <
> > > > > > john.p.donnelly@oracle.com> wrote:
> > > > > > > Hi .  See below !
> > > > > > > 
> > > > > > > > On Jun 1, 2020, at 4:02 PM, Bhupesh Sharma <bhsharma@redhat.com>
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > Hi John,
> > > > > > > > 
> > > > > > > > On Tue, Jun 2, 2020 at 1:01 AM John Donnelly <
> > > > > > > > John.P.donnelly@oracle.com> wrote:
> > > > > > > > > Hi,
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 6/1/20 7:02 AM, Prabhakar Kushwaha wrote:
> > > > > > > > > > Hi Chen,
> > > > > > > > > > 
> > > > > > > > > > On Thu, May 21, 2020 at 3:05 PM Chen Zhou <
> > > > > > > > > > chenzhou10@huawei.com> wrote:
> > > > > > > > > > > This patch series enable reserving crashkernel above 4G in
> > > > > > > > > > > arm64.
> > > > > > > > > > > 
> > > > > > > > > > > There are following issues in arm64 kdump:
> > > > > > > > > > > 1. We use crashkernel=X to reserve crashkernel below 4G,
> > > > > > > > > > > which will fail
> > > > > > > > > > > when there is no enough low memory.
> > > > > > > > > > > 2. Currently, crashkernel=Y@X can be used to reserve
> > > > > > > > > > > crashkernel above 4G,
> > > > > > > > > > > in this case, if swiotlb or DMA buffers are required,
> > > > > > > > > > > crash dump kernel
> > > > > > > > > > > will boot failure because there is no low memory available
> > > > > > > > > > > for allocation.
> > > > > > > > > > > 
> > > > > > > > > > We are getting "warn_alloc" [1] warning during boot of kdump
> > > > > > > > > > kernel
> > > > > > > > > > with bootargs as [2] of primary kernel.
> > > > > > > > > > This error observed on ThunderX2  ARM64 platform.
> > > > > > > > > > 
> > > > > > > > > > It is observed with latest upstream tag (v5.7-rc3) with this
> > > > > > > > > > patch set
> > > > > > > > > > and 
> > > > > > > > > > 
https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbiIAAlzu$
> > > > > > > > > > Also **without** this patch-set
> > > > > > > > > > "
> > > > > > > > > > 
https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$
> > > > > > > > > > "
> > > > > > > > > > 
> > > > > > > > > > This issue comes whenever crashkernel memory is reserved
> > > > > > > > > > after 0xc000_0000.
> > > > > > > > > > More details discussed earlier in
> > > > > > > > > > 
https://urldefense.com/v3/__https://www.spinics.net/lists/arm-kernel/msg806882.html__;!!GqivPVa7Brio!LnTSARkCt0V0FozR0KmqooaH5ADtdXvs3mPdP3KRVqALmvSK2VmCkIPIhsaxbjC6ujMA$
  without
> > > > > > > > > > any
> > > > > > > > > > solution
> > > > > > > > > > 
> > > > > > > > > > This patch-set is expected to solve similar kind of issue.
> > > > > > > > > > i.e. low memory is only targeted for DMA, swiotlb; So above
> > > > > > > > > > mentioned
> > > > > > > > > > observation should be considered/fixed. .
> > > > > > > > > > 
> > > > > > > > > > --pk
> > > > > > > > > > 
> > > > > > > > > > [1]
> > > > > > > > > > [   30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
> > > > > > > > > > TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
> > > > > > > > > > [   30.367696] NET: Registered protocol family 16
> > > > > > > > > > [   30.369973] swapper/0: page allocation failure: order:6,
> > > > > > > > > > mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
> > > > > > > > > > [   30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> > > > > > > > > > 5.7.0-rc3+ #121
> > > > > > > > > > [   30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
> > > > > > > > > > TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
> > > > > > > > > > [   30.369984] Call trace:
> > > > > > > > > > [   30.369989]  dump_backtrace+0x0/0x1f8
> > > > > > > > > > [   30.369991]  show_stack+0x20/0x30
> > > > > > > > > > [   30.369997]  dump_stack+0xc0/0x10c
> > > > > > > > > > [   30.370001]  warn_alloc+0x10c/0x178
> > > > > > > > > > [   30.370004]  __alloc_pages_slowpath.constprop.111+0xb10/0
> > > > > > > > > > xb50
> > > > > > > > > > [   30.370006]  __alloc_pages_nodemask+0x2b4/0x300
> > > > > > > > > > [   30.370008]  alloc_page_interleave+0x24/0x98
> > > > > > > > > > [   30.370011]  alloc_pages_current+0xe4/0x108
> > > > > > > > > > [   30.370017]  dma_atomic_pool_init+0x44/0x1a4
> > > > > > > > > > [   30.370020]  do_one_initcall+0x54/0x228
> > > > > > > > > > [   30.370027]  kernel_init_freeable+0x228/0x2cc
> > > > > > > > > > [   30.370031]  kernel_init+0x1c/0x110
> > > > > > > > > > [   30.370034]  ret_from_fork+0x10/0x18
> > > > > > > > > > [   30.370036] Mem-Info:
> > > > > > > > > > [   30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
> > > > > > > > > > [   30.370064]  active_file:0 inactive_file:0
> > > > > > > > > > isolated_file:0
> > > > > > > > > > [   30.370064]  unevictable:0 dirty:0 writeback:0 unstable:0
> > > > > > > > > > [   30.370064]  slab_reclaimable:34 slab_unreclaimable:4438
> > > > > > > > > > [   30.370064]  mapped:0 shmem:0 pagetables:14 bounce:0
> > > > > > > > > > [   30.370064]  free:1537719 free_pcp:219 free_cma:0
> > > > > > > > > > [   30.370070] Node 0 active_anon:0kB inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > isolated(anon):0kB
> > > > > > > > > > isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB
> > > > > > > > > > shmem:0kB
> > > > > > > > > > shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB
> > > > > > > > > > writeback_tmp:0kB
> > > > > > > > > > unstable:0kB all_unreclaimable? no
> > > > > > > > > > [   30.370073] Node 1 active_anon:0kB inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > isolated(anon):0kB
> > > > > > > > > > isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB
> > > > > > > > > > shmem:0kB
> > > > > > > > > > shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB
> > > > > > > > > > writeback_tmp:0kB
> > > > > > > > > > unstable:0kB all_unreclaimable? no
> > > > > > > > > > [   30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
> > > > > > > > > > reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > writepending:0kB
> > > > > > > > > > present:128kB managed:0kB mlocked:0kB kernel_stack:0kB
> > > > > > > > > > pagetables:0kB
> > > > > > > > > > bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
> > > > > > > > > > [   30.370084] lowmem_reserve[]: 0 250 6063 6063
> > > > > > > > > > [   30.370090] Node 0 DMA32 free:256000kB min:408kB
> > > > > > > > > > low:664kB
> > > > > > > > > > high:920kB reserved_highatomic:0KB active_anon:0kB
> > > > > > > > > > inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > writepending:0kB
> > > > > > > > > > present:269700kB managed:256000kB mlocked:0kB
> > > > > > > > > > kernel_stack:0kB
> > > > > > > > > > pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB
> > > > > > > > > > free_cma:0kB
> > > > > > > > > > [   30.370094] lowmem_reserve[]: 0 0 5813 5813
> > > > > > > > > > [   30.370100] Node 0 Normal free:5894876kB min:9552kB
> > > > > > > > > > low:15504kB
> > > > > > > > > > high:21456kB reserved_highatomic:0KB active_anon:0kB
> > > > > > > > > > inactive_anon:0kB
> > > > > > > > > > active_file:0kB inactive_file:0kB unevictable:0kB
> > > > > > > > > > writepending:0kB
> > > > > > > > > > present:8388608kB managed:5953112kB mlocked:0kB
> > > > > > > > > > kernel_stack:21672kB
> > > > > > > > > > pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB
> > > > > > > > > > free_cma:0kB
> > > > > > > > > > [   30.370104] lowmem_reserve[]: 0 0 0 0
> > > > > > > > > > [   30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB
> > > > > > > > > > 0*128kB
> > > > > > > > > > 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
> > > > > > > > > > [   30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB
> > > > > > > > > > 0*64kB 0*128kB
> > > > > > > > > > 0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) =
> > > > > > > > > > 256000kB
> > > > > > > > > > [   30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB
> > > > > > > > > > (UE) 3*32kB
> > > > > > > > > > (UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME)
> > > > > > > > > > 3*1024kB (ME)
> > > > > > > > > > 3*2048kB (UME) 1436*4096kB (M) = 5893600kB
> > > > > > > > > > [   30.370129] Node 0 hugepages_total=0 hugepages_free=0
> > > > > > > > > > hugepages_surp=0 hugepages_size=1048576kB
> > > > > > > > > > [   30.370130] 0 total pagecache pages
> > > > > > > > > > [   30.370132] 0 pages in swap cache
> > > > > > > > > > [   30.370134] Swap cache stats: add 0, delete 0, find 0/0
> > > > > > > > > > [   30.370135] Free swap  = 0kB
> > > > > > > > > > [   30.370136] Total swap = 0kB
> > > > > > > > > > [   30.370137] 2164609 pages RAM
> > > > > > > > > > [   30.370139] 0 pages HighMem/MovableOnly
> > > > > > > > > > [   30.370140] 612331 pages reserved
> > > > > > > > > > [   30.370141] 0 pages hwpoisoned
> > > > > > > > > > [   30.370143] DMA: failed to allocate 256 KiB pool for
> > > > > > > > > > atomic
> > > > > > > > > > coherent allocation
> > > > > > > > > 
> > > > > > > > > During my testing I saw the same error and Chen's  solution
> > > > > > > > > corrected it .
> > > > > > > > Which combination you are using on your side? I am using
> > > > > > > > Prabhakar's
> > > > > > > > suggested environment and can reproduce the issue
> > > > > > > > with or without Chen's crashkernel support above 4G patchset.
> > > > > > > > 
> > > > > > > > I am also using a ThunderX2 platform with latest makedumpfile
> > > > > > > > code and
> > > > > > > > kexec-tools (with the suggested patch
> > > > > > > > <
> > > > > > > > 
https://urldefense.com/v3/__https://lists.infradead.org/pipermail/kexec/2020-May/025128.html__;!!GqivPVa7Brio!J6lUig58-Gw6TKZnEEYzEeSU36T-1SqlB1kImU00xtX_lss5Tx-JbUmLE9TJC3foXBLg$
> > > > > > > > >).
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > > Bhupesh
> > > > > > > 
> > > > > > > I did this activity 5 months ago and I have moved on to other
> > > > > > > activities. My DMA failures were related to PCI devices that could
> > > > > > > not be enumerated because  low-DMA space was not  available when
> > > > > > > crashkernel was moved above 4G; I don’t recall the exact platform.
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > For this failure ,
> > > > > > > 
> > > > > > > > > > DMA: failed to allocate 256 KiB pool for atomic
> > > > > > > > > > coherent allocation
> > > > > > > 
> > > > > > > Is due to :
> > > > > > > 
> > > > > > > 
> > > > > > > 3618082c
> > > > > > > ("arm64 use both ZONE_DMA and ZONE_DMA32")
> > > > > > > 
> > > > > > > With the introduction of ZONE_DMA to support the Raspberry DMA
> > > > > > > region below 1G, the crashkernel is placed in the upper 4G
> > > > > > > ZONE_DMA_32 region. Since the crashkernel does not have access
> > > > > > > to the ZONE_DMA region, it prints out call trace during bootup.
> > > > > > > 
> > > > > > > It is due to having this CONFIG item  ON  :
> > > > > > > 
> > > > > > > 
> > > > > > > CONFIG_ZONE_DMA=y
> > > > > > > 
> > > > > > > Turning off ZONE_DMA fixes a issue and Raspberry PI 4 will
> > > > > > > use the device tree to specify memory below 1G.
> > > > > > > 
> > > > > > > 
> > > > > > Disabling ZONE_DMA is temporary solution.  We may need proper
> > > > > > solution
> > > > > 
> > > > > Perhaps the Raspberry platform configuration dependencies need
> > > > > separated  from “server class” Arm  equipment ?  Or auto-configured on
> > > > > boot ?  Consult an expert ;-)
> > > > > 
> > > > > 
> > > > > 
> > > > > > > I would like to see Chen’s feature added , perhaps as
> > > > > > > EXPERIMENTAL,  so we can get some configuration testing done on
> > > > > > > it.   It corrects having a DMA zone in low memory while crash-
> > > > > > > kernel is above 4GB.  This has been going on for a year now.
> > > > > > I will also like this patch to be added in Linux as early as
> > > > > > possible.
> > > > > > 
> > > > > > Issue mentioned by me happens with or without this patch.
> > > > > > 
> > > > > > This patch-set can consider fixing because it uses low memory for
> > > > > > DMA
> > > > > > & swiotlb only.
> > > > > > We can consider restricting crashkernel within the required range
> > > > > > like below
> > > > > > 
> > > > > > diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> > > > > > index 7f9e5a6dc48c..bd67b90d35bd 100644
> > > > > > --- a/kernel/crash_core.c
> > > > > > +++ b/kernel/crash_core.c
> > > > > > @@ -354,7 +354,7 @@ int __init reserve_crashkernel_low(void)
> > > > > >                       return 0;
> > > > > >       }
> > > > > > 
> > > > > > -       low_base = memblock_find_in_range(0, 1ULL << 32, low_size,
> > > > > > CRASH_ALIGN);
> > > > > > +       low_base = memblock_find_in_range(0,0xc0000000, low_size,
> > > > > > CRASH_ALIGN);
> > > > > >       if (!low_base) {
> > > > > >               pr_err("Cannot reserve %ldMB crashkernel low memory,
> > > > > > please try smaller size.\n",
> > > > > >                      (unsigned long)(low_size >> 20));
> > > > > > 
> > > > > > 
> > > > >    I suspect  0xc0000000  would need to be a CONFIG item  and not
> > > > > hard-coded.
> > > > > 
> > > > if you consider this as valid change,  can you please incorporate as
> > > > part of your patch-set.
> > > 
> > > After commit 1a8e1cef7 ("arm64: use both ZONE_DMA and ZONE_DMA32"),the 0-
> > > 4G memory is splited
> > > to DMA [mem 0x0000000000000000-0x000000003fffffff] and DMA32 [mem
> > > 0x0000000040000000-0x00000000ffffffff] on arm64.
> > > 
> > > From the above discussion, on your platform, the low crashkernel fall in
> > > DMA32 region, but your environment needs to access DMA
> > > region, so there is the call trace.
> > > 
> > > I have a question, why do you choose 0xc0000000 here?
> > > 
> > > Besides, this is common code, we also need to consider about x86.
> > > 
> > 
> >  + nsaenzjulienne@suse.de

Thanks for adding me to the conversation, and sorry for the headaches.

> > 
> >   Exactly .  This is why it needs to be a CONFIG option for  Raspberry
> > ..,  or device tree option.
> > 
> > 
> >   We could revert 1a8e1cef7 since it broke  Arm kdump too.
> 
> Well, unfortunately the patch for commit 1a8e1cef7603 ("arm64: use
> both ZONE_DMA and ZONE_DMA32") was not Cc'ed to the kexec mailing
> list, thus we couldn't get many eyes on it for a thorough review from
> kexec/kdump p-o-v.
> 
> Also we historically never had distinction in common arch code on the
> basis of the intended end use-case: embedded, server or automotive, so
> I am not sure introducing a Raspberry specific CONFIG option would be
> a good idea.

+1

From the distros perspective it's very important to keep a single kernel image.

> So, rather than reverting the patch, we can look at addressing the
> same properly this time - especially from a kdump p-o-v.
> This issue has been reported by some Red Hat arm64 partners with
> upstream kernel also and as we have noticed in the past as well,
> hardcoding the placement of the crashkernel base address (unless the
> base address is specified by a crashkernel=X@Y like bootargs) is also
> not a portable suggestion.
> 
> I am working on a possible fix and will have more updates on the same
> in a day-or-two.

Please keep me in the loop, we've also had issues pointing to this reported by
SUSE partners. I can do some testing both on the RPi4 and on big servers that
need huge crashkernel sizes.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v2 0/3] media: rockchip: Introduce driver for the camera interface on PX30
From: Tomasz Figa @ 2020-06-04 17:04 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Maxime Chevallier, Helen Koike, Dafna Hirschfeld,
	Mauro Carvalho Chehab, Robin Murphy, Rob Herring, Mark Rutland,
	Hans Verkuil, Linux Media Mailing List, linux-devicetree
In-Reply-To: <1779471.kMuJgyiE6z@diego>

On Mon, Jun 1, 2020 at 11:38 PM Heiko Stübner <heiko@sntech.de> wrote:
>
> Hi Tomasz,
>
> Am Montag, 1. Juni 2020, 20:45:14 CEST schrieb Tomasz Figa:
> > On Fri, May 29, 2020 at 3:04 PM Maxime Chevallier
> > <maxime.chevallier@bootlin.com> wrote:
> > >
> > > Hello everyone,
> > >
> > > Here's a V2 of the series adding very basic support for the camera interface on
> > > the Rockchip PX30 SoC.
> > >
> > > Thanks to everyone that commented on the first series, your reviews were
> > > very helpful :)
> > >
> > > This Camera Interface is also supported on other Rockchip SoC such as
> > > the RK1808, RK3128, RK3288 and RK3288, but for now I've only been able to
> > > test it on the PX30, using a PAL format.
> >
> > How does this hardware relate to the one handled by the rkisp1 driver
> > that is available under staging/media/rkisp1? It was written with
> > RK3399 in mind, but I have a loose recollection that the hardware in
> > RK3288 was roughly the same.
>
> (un-)educated guess would be that the rk3288 has both.
>
> When introducing new IPs Rockchip often keeps the previous incarnation
> around - probably as a fallback.
>
> From a bit of digging around manuals and vendor-dtsi [0] I found:
>
> in rk3288.dtsi both:
> - isp: isp@ff910000
> - cif_isp0: cif_isp@ff910000
>
> - grf_con_disable_isp in GRF_SOC_CON6
> - dphy_rx1_src_sel (1: isp, 0: csi host) in GRF_SOC_CON14
>

Makes sense. Thanks!

Right now the rkisp1 driver doesn't support rk3288, because we didn't
have any way to test it, but it shouldn't require much changes to do
so. If that happens, I wonder how one would select between the two
hardware blocks?

Best regards,
Tomasz

>
> Heiko
>
>
> [0] https://github.com/rockchip-linux/kernel/blob/develop-4.4/arch/arm/boot/dts/rk3288.dtsi
>
>
> > +Helen Koike +Dafna Hirschfeld working on the rkisp1 driver.
> >
> > Best regards,
> > Tomasz
> >
> > >
> > > This driver is mostly based on the driver found in Rockchip's BSP, that
> > > has been trimmed down to support the set of features that I was able to test,
> > > that is pretty much a very basic one-frame capture and video streaming
> > > with GStreamer.
> > >
> > > This first draft only supports the Parallel interface, although the
> > > controller has support for BT656 and CSI2.
> > >
> > > Finally, this controller has an iommu that could be used in this driver,
> > > but as of today I've not been able to get it to work.
> > >
> > > Any review is welcome.
> > >
> > > Thanks,
> > >
> > > Maxime
> > >
> > > --- Changes since V1 ---
> > >
> > >  - Took reviews from Rob, Hans, Robin and Heiko into account :
> > >   - Renamed the clocks in the binding
> > >   - Fixed the DT schema compiling
> > >   - Fixed a few typos
> > >   - Used the clk bulk API
> > >   - Used the reset array API
> > >   - Changed a few helpers for more suitable ones
> > >   - Rebased on 5.7-rc7
> > >
> > >
> > >
> > > Maxime Chevallier (3):
> > >   media: dt-bindings: media: Document Rockchip CIF bindings
> > >   media: rockchip: Introduce driver for Rockhip's camera interface
> > >   arm64: dts: rockchip: Add the camera interface description of the PX30
> > >
> > >  .../bindings/media/rockchip-cif.yaml          |  100 ++
> > >  arch/arm64/boot/dts/rockchip/px30.dtsi        |   12 +
> > >  drivers/media/platform/Kconfig                |   13 +
> > >  drivers/media/platform/Makefile               |    1 +
> > >  drivers/media/platform/rockchip/cif/Makefile  |    3 +
> > >  drivers/media/platform/rockchip/cif/capture.c | 1170 +++++++++++++++++
> > >  drivers/media/platform/rockchip/cif/dev.c     |  358 +++++
> > >  drivers/media/platform/rockchip/cif/dev.h     |  213 +++
> > >  drivers/media/platform/rockchip/cif/regs.h    |  256 ++++
> > >  9 files changed, 2126 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/rockchip-cif.yaml
> > >  create mode 100644 drivers/media/platform/rockchip/cif/Makefile
> > >  create mode 100644 drivers/media/platform/rockchip/cif/capture.c
> > >  create mode 100644 drivers/media/platform/rockchip/cif/dev.c
> > >  create mode 100644 drivers/media/platform/rockchip/cif/dev.h
> > >  create mode 100644 drivers/media/platform/rockchip/cif/regs.h
> > >
> > > --
> > > 2.25.4
> > >
> >
>
>
>
>

^ permalink raw reply

* Re: [PATCH V2 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support
From: Matthias Kaehlcke @ 2020-06-04 17:09 UTC (permalink / raw)
  To: Pradeep P V K
  Cc: bjorn.andersson, adrian.hunter, robh+dt, ulf.hansson, vbadigan,
	sboyd, georgi.djakov, linux-mmc, linux-kernel, linux-arm-msm,
	devicetree, linux-mmc-owner, rnayak, sibis, matthias
In-Reply-To: <1591269283-24084-2-git-send-email-ppvk@codeaurora.org>

On Thu, Jun 04, 2020 at 04:44:42PM +0530, Pradeep P V K wrote:
> Interconnect bandwidth scaling support is now added as a
> part of OPP [1]. So, make sure interconnect driver is ready
> before handling interconnect scaling.
> 
> This change is based on
> [1] [Patch v8] Introduce OPP bandwidth bindings
> (https://lkml.org/lkml/2020/5/12/493)
> 
> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
> for dev_pm_opp_of_add_table()
> (https://lkml.org/lkml/2020/5/5/491)
> 
> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index b277dd7..a13ff1b 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -14,6 +14,7 @@
>  #include <linux/slab.h>
>  #include <linux/iopoll.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/interconnect.h>
>  
>  #include "sdhci-pltfm.h"
>  #include "cqhci.h"
> @@ -2070,6 +2071,18 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  	}
>  	msm_host->bulk_clks[0].clk = clk;
>  
> +	/* Make sure that ICC driver is ready for interconnect bandwdith
> +	 * scaling before registering the device for OPP.
> +	 */
> +	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
> +	if (ret) {
> +		if (ret == -EPROBE_DEFER)
> +			dev_info(&pdev->dev, "defer icc path: %d\n", ret);

I already commented on this on v1:

  This log seems to add little more than noise, or are there particular reasons
  why it is useful in this driver? Most drivers just return silently in case of
  deferred probing.

If you think the log is really needed please explain why.

^ permalink raw reply

* Re: [PATCH v3 004/105] clk: bcm: Add BCM2711 DVP driver
From: Nicolas Saenz Julienne @ 2020-06-04 17:26 UTC (permalink / raw)
  To: Maxime Ripard, Eric Anholt
  Cc: dri-devel, linux-rpi-kernel, bcm-kernel-feedback-list,
	linux-arm-kernel, linux-kernel, Dave Stevenson, Tim Gover,
	Phil Elwell, Michael Turquette, Stephen Boyd, Rob Herring,
	linux-clk, devicetree
In-Reply-To: <6615a61b8af240e3d10f8890e4b2462ccdaac9b9.1590594512.git-series.maxime@cerno.tech>

[-- Attachment #1: Type: text/plain, Size: 6101 bytes --]

Hi Maxime,

On Wed, 2020-05-27 at 17:47 +0200, Maxime Ripard wrote:
> The HDMI block has a block that controls clocks and reset signals to the
> HDMI0 and HDMI1 controllers.

Why not having two separate drivers?

> Let's expose that through a clock driver implementing a clock and reset
> provider.
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-clk@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Reviewed-by: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/clk/bcm/Kconfig           |  11 +++-
>  drivers/clk/bcm/Makefile          |   1 +-
>  drivers/clk/bcm/clk-bcm2711-dvp.c | 127 +++++++++++++++++++++++++++++++-
>  3 files changed, 139 insertions(+)
>  create mode 100644 drivers/clk/bcm/clk-bcm2711-dvp.c
> 
> diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
> index 8c83977a7dc4..784f12c72365 100644
> --- a/drivers/clk/bcm/Kconfig
> +++ b/drivers/clk/bcm/Kconfig
> @@ -1,4 +1,15 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> +
> +config CLK_BCM2711_DVP
> +	tristate "Broadcom BCM2711 DVP support"
> +	depends on ARCH_BCM2835 ||COMPILE_TEST
> +	depends on COMMON_CLK
> +	default ARCH_BCM2835
> +	select RESET_SIMPLE
> +	help
> +	  Enable common clock framework support for the Broadcom BCM2711
> +	  DVP Controller.
> +
>  config CLK_BCM2835
>  	bool "Broadcom BCM2835 clock support"
>  	depends on ARCH_BCM2835 || ARCH_BRCMSTB || COMPILE_TEST
> diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
> index 0070ddf6cdd2..2c1349062147 100644
> --- a/drivers/clk/bcm/Makefile
> +++ b/drivers/clk/bcm/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona-setup.o
>  obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm281xx.o
>  obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
>  obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o
> clk-iproc-asiu.o
> +obj-$(CONFIG_CLK_BCM2835)	+= clk-bcm2711-dvp.o
>  obj-$(CONFIG_CLK_BCM2835)	+= clk-bcm2835.o
>  obj-$(CONFIG_CLK_BCM2835)	+= clk-bcm2835-aux.o
>  obj-$(CONFIG_CLK_RASPBERRYPI)	+= clk-raspberrypi.o
> diff --git a/drivers/clk/bcm/clk-bcm2711-dvp.c b/drivers/clk/bcm/clk-bcm2711-
> dvp.c
> new file mode 100644
> index 000000000000..c1c4b5857d32
> --- /dev/null
> +++ b/drivers/clk/bcm/clk-bcm2711-dvp.c
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright 2020 Cerno
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/reset/reset-simple.h>
> +
> +#define DVP_HT_RPI_SW_INIT	0x04
> +#define DVP_HT_RPI_MISC_CONFIG	0x08
> +
> +#define NR_CLOCKS	2
> +#define NR_RESETS	6
> +
> +struct clk_dvp {
> +	struct clk_hw_onecell_data	*data;
> +	struct reset_simple_data	reset;
> +};
> +
> +static const struct clk_parent_data clk_dvp_parent = {
> +	.index	= 0,
> +};
> +
> +static int clk_dvp_probe(struct platform_device *pdev)
> +{
> +	struct clk_hw_onecell_data *data;
> +	struct resource *res;
> +	struct clk_dvp *dvp;
> +	void __iomem *base;
> +	int ret;
> +
> +	dvp = devm_kzalloc(&pdev->dev, sizeof(*dvp), GFP_KERNEL);
> +	if (!dvp)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, dvp);
> +
> +	dvp->data = devm_kzalloc(&pdev->dev,
> +				 struct_size(dvp->data, hws, NR_CLOCKS),
> +				 GFP_KERNEL);
> +	if (!dvp->data)
> +		return -ENOMEM;
> +	data = dvp->data;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	base = devm_ioremap_resource(&pdev->dev, res);

I think the cool function to use these days is
devm_platform_get_and_ioremap_resource().

Regards,
Nicolas

> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	dvp->reset.rcdev.owner = THIS_MODULE;
> +	dvp->reset.rcdev.nr_resets = NR_RESETS;
> +	dvp->reset.rcdev.ops = &reset_simple_ops;
> +	dvp->reset.rcdev.of_node = pdev->dev.of_node;
> +	dvp->reset.membase = base + DVP_HT_RPI_SW_INIT;
> +	spin_lock_init(&dvp->reset.lock);
> +
> +	ret = reset_controller_register(&dvp->reset.rcdev);
> +	if (ret)
> +		return ret;
> +
> +	data->hws[0] = clk_hw_register_gate_parent_data(&pdev->dev,
> +							"hdmi0-108MHz",
> +							&clk_dvp_parent, 0,
> +							base +
> DVP_HT_RPI_MISC_CONFIG, 3,
> +							CLK_GATE_SET_TO_DISABLE,
> +							&dvp->reset.lock);
> +	if (IS_ERR(data->hws[0])) {
> +		ret = PTR_ERR(data->hws[0]);
> +		goto unregister_reset;
> +	}
> +
> +	data->hws[1] = clk_hw_register_gate_parent_data(&pdev->dev,
> +							"hdmi1-108MHz",
> +							&clk_dvp_parent, 0,
> +							base +
> DVP_HT_RPI_MISC_CONFIG, 4,
> +							CLK_GATE_SET_TO_DISABLE,
> +							&dvp->reset.lock);
> +	if (IS_ERR(data->hws[1])) {
> +		ret = PTR_ERR(data->hws[1]);
> +		goto unregister_clk0;
> +	}
> +
> +	data->num = NR_CLOCKS;
> +	ret = of_clk_add_hw_provider(pdev->dev.of_node, of_clk_hw_onecell_get,

> +				     data);
> +	if (ret)
> +		goto unregister_clk1;
> +
> +	return 0;
> +
> +unregister_clk1:
> +	clk_hw_unregister_gate(data->hws[1]);
> +
> +unregister_clk0:
> +	clk_hw_unregister_gate(data->hws[0]);
> +
> +unregister_reset:
> +	reset_controller_unregister(&dvp->reset.rcdev);
> +	return ret;
> +};
> +
> +static int clk_dvp_remove(struct platform_device *pdev)
> +{
> +	struct clk_dvp *dvp = platform_get_drvdata(pdev);
> +	struct clk_hw_onecell_data *data = dvp->data;
> +
> +	clk_hw_unregister_gate(data->hws[1]);
> +	clk_hw_unregister_gate(data->hws[0]);
> +	reset_controller_unregister(&dvp->reset.rcdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id clk_dvp_dt_ids[] = {
> +	{ .compatible = "brcm,brcm2711-dvp", },
> +	{ /* sentinel */ }
> +};
> +
> +static struct platform_driver clk_dvp_driver = {
> +	.probe	= clk_dvp_probe,
> +	.remove	= clk_dvp_remove,
> +	.driver	= {
> +		.name		= "brcm2711-dvp",
> +		.of_match_table	= clk_dvp_dt_ids,
> +	},
> +};
> +module_platform_driver(clk_dvp_driver);


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH V1 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support
From: Sibi Sankar @ 2020-06-04 17:55 UTC (permalink / raw)
  To: ppvk
  Cc: bjorn.andersson, adrian.hunter, robh+dt, ulf.hansson, vbadigan,
	sboyd, georgi.djakov, mka, linux-mmc, linux-kernel, linux-arm-msm,
	devicetree, linux-mmc-owner, rnayak, matthias,
	linux-arm-msm-owner, linux-kernel-owner
In-Reply-To: <8865e3b00fce4f28264b60cd498fcf02@codeaurora.org>

On 2020-06-04 16:43, ppvk@codeaurora.org wrote:
> Hi Sibi,
> 
> Thanks for the review!!
> 
> On 2020-06-03 17:22, Sibi Sankar wrote:
>> Hey Pradeep,
>> Thanks for the patch.
>> 
>> On 2020-06-03 14:39, Pradeep P V K wrote:
>>> Interconnect bandwidth scaling support is now added as a
>>> part of OPP [1]. So, make sure interconnect driver is ready
>>> before handling interconnect scaling.
>>> 
>>> This change is based on
>>> [1] [Patch v8] Introduce OPP bandwidth bindings
>>> (https://lkml.org/lkml/2020/5/12/493)
>>> 
>>> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
>>> for dev_pm_opp_of_add_table()
>>> (https://lkml.org/lkml/2020/5/5/491)
>>> 
>>> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
>>> ---
>>>  drivers/mmc/host/sdhci-msm.c | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>> 
>>> diff --git a/drivers/mmc/host/sdhci-msm.c 
>>> b/drivers/mmc/host/sdhci-msm.c
>>> index b277dd7..bf95484 100644
>>> --- a/drivers/mmc/host/sdhci-msm.c
>>> +++ b/drivers/mmc/host/sdhci-msm.c
>>> @@ -14,6 +14,7 @@
>>>  #include <linux/slab.h>
>>>  #include <linux/iopoll.h>
>>>  #include <linux/regulator/consumer.h>
>>> +#include <linux/interconnect.h>
>>> 
>>>  #include "sdhci-pltfm.h"
>>>  #include "cqhci.h"
>>> @@ -1999,6 +2000,7 @@ static int sdhci_msm_probe(struct 
>>> platform_device *pdev)
>>>  	struct sdhci_pltfm_host *pltfm_host;
>>>  	struct sdhci_msm_host *msm_host;
>>>  	struct clk *clk;
>>> +	struct icc_path *sdhc_path;
>>>  	int ret;
>>>  	u16 host_version, core_minor;
>>>  	u32 core_version, config;
>>> @@ -2070,6 +2072,20 @@ static int sdhci_msm_probe(struct 
>>> platform_device *pdev)
>>>  	}
>>>  	msm_host->bulk_clks[0].clk = clk;
>>> 
>>> +	/* Make sure that ICC driver is ready for interconnect bandwdith
>>> +	 * scaling before registering the device for OPP.
>>> +	 */
>>> +	sdhc_path = of_icc_get(&pdev->dev, NULL);
>>> +	ret = PTR_ERR_OR_ZERO(sdhc_path);
>>> +	if (ret) {
>>> +		if (ret == -EPROBE_DEFER)
>>> +			dev_info(&pdev->dev, "defer icc path: %d\n", ret);
>>> +		else
>>> +			dev_err(&pdev->dev, "failed to get icc path:%d\n", ret);
>>> +		goto bus_clk_disable;
>>> +	}
>>> +	icc_put(sdhc_path);
>> 
>> ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
>> 
>> since there are multiple paths
>> described in the bindings you
>> should use ^^ instead and you
>> can drop temporary path as well.
>> 
> Ok. of_icc_get() used above is only to test if ICC driver is ready or
> not. I'm not
> really using the multiple paths here. Anyhow i will use
> dev_pm_opp_of_find_icc_paths()
> to get rid of some extra lines of code.

Using dev_pm_opp_of_find_icc_paths
with NULL passed acts as a way of
validating all the paths specified
in the device and also validates if
the opp-table has bw related bindings
as well.

> 
>>> +
>>>  	msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
>>>  	if (IS_ERR(msm_host->opp_table)) {
>>>  		ret = PTR_ERR(msm_host->opp_table);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply

* Re: [V9, 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver
From: Tomasz Figa @ 2020-06-04 18:05 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Dongchun Zhu, Linus Walleij, Bartosz Golaszewski,
	Mauro Carvalho Chehab, Andy Shevchenko, Rob Herring, Mark Rutland,
	Nicolas Boichat, Matthias Brugger, Cao Bing Bu, srv_heupstream,
	moderated list:ARM/Mediatek SoC support,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>,,
	Sj Huang, Linux Media Mailing List, linux-devicetree, Louis Kuo,
	Shengnan Wang (王圣男)
In-Reply-To: <20200604092616.GJ16711@paasikivi.fi.intel.com>

On Thu, Jun 4, 2020 at 11:26 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Dongchun,
>
> On Thu, Jun 04, 2020 at 10:14:05AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz, Sakari, and sirs,
> >
> > Could anyone help to review this patch?
> >
> > On Sat, 2020-05-23 at 16:41 +0800, Dongchun Zhu wrote:
> > > Add a V4L2 sub-device driver for OV02A10 image sensor.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > >  MAINTAINERS                 |    1 +
> > >  drivers/media/i2c/Kconfig   |   13 +
> > >  drivers/media/i2c/Makefile  |    1 +
> > >  drivers/media/i2c/ov02a10.c | 1025 +++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 1040 insertions(+)
> > >  create mode 100644 drivers/media/i2c/ov02a10.c
> > >
> >
> > [snip]
> >
> > > +static int ov02a10_probe(struct i2c_client *client)
> > > +{
> > > +   struct device *dev = &client->dev;
> > > +   struct ov02a10 *ov02a10;
> > > +   unsigned int rotation;
> > > +   unsigned int clock_lane_tx_speed;
> > > +   unsigned int i;
> > > +   int ret;
> > > +
> > > +   ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > > +   if (!ov02a10)
> > > +           return -ENOMEM;
> > > +
> > > +   ret = ov02a10_check_hwcfg(dev, ov02a10);
> > > +   if (ret) {
> > > +           dev_err(dev, "failed to check HW configuration: %d", ret);
> > > +           return ret;
> > > +   }
> > > +
> > > +   v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > > +   ov02a10->mipi_clock_tx_speed = OV02A10_MIPI_TX_SPEED_DEFAULT;
> > > +   ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > > +
> > > +   /* Optional indication of physical rotation of sensor */
> > > +   ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation", &rotation);
> > > +   if (!ret && rotation == 180) {
> > > +           ov02a10->upside_down = true;
> > > +           ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > > +   }
> > > +
> > > +   /* Optional indication of mipi TX speed */
> > > +   ret = fwnode_property_read_u32(dev_fwnode(dev), "ovti,mipi-tx-speed",
> > > +                                  &clock_lane_tx_speed);
> > > +
> > > +   if (!ret)
> > > +           ov02a10->mipi_clock_tx_speed = clock_lane_tx_speed;
> > > +
> > > +   /* Get system clock (eclk) */
> > > +   ov02a10->eclk = devm_clk_get(dev, "eclk");
> > > +   if (IS_ERR(ov02a10->eclk)) {
> > > +           ret = PTR_ERR(ov02a10->eclk);
> > > +           dev_err(dev, "failed to get eclk %d\n", ret);
> > > +           return ret;
> > > +   }
> > > +
> > > +   ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> > > +                                  &ov02a10->eclk_freq);
> > > +   if (ret) {
> > > +           dev_err(dev, "failed to get eclk frequency\n");
> > > +           return ret;
> > > +   }
> > > +
> > > +   ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> > > +   if (ret) {
> > > +           dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> > > +           return ret;
> > > +   }
> > > +
> > > +   if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> > > +           dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> > > +                    ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> > > +           return -EINVAL;
> > > +   }
> > > +
> > > +   ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> > > +   if (IS_ERR(ov02a10->pd_gpio)) {
> > > +           ret = PTR_ERR(ov02a10->pd_gpio);
> > > +           dev_err(dev, "failed to get powerdown-gpios %d\n", ret);
> > > +           return ret;
> > > +   }
> > > +
> > > +   ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > +   if (IS_ERR(ov02a10->n_rst_gpio)) {
> > > +           ret = PTR_ERR(ov02a10->n_rst_gpio);
> > > +           dev_err(dev, "failed to get reset-gpios %d\n", ret);
> > > +           return ret;
> > > +   }
> > > +
> > > +   for (i = 0; i < ARRAY_SIZE(ov02a10_supply_names); i++)
> > > +           ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > > +
> > > +   ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov02a10_supply_names),
> > > +                                 ov02a10->supplies);
> > > +   if (ret) {
> > > +           dev_err(dev, "failed to get regulators\n");
> > > +           return ret;
> > > +   }
> > > +
> > > +   mutex_init(&ov02a10->mutex);
> > > +   ov02a10->cur_mode = &supported_modes[0];
> > > +   ret = ov02a10_initialize_controls(ov02a10);
> > > +   if (ret) {
> > > +           dev_err(dev, "failed to initialize controls\n");
> > > +           goto err_destroy_mutex;
> > > +   }
> > > +
> > > +   ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > +   ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > > +   ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > +   ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > +   ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > > +   if (ret < 0) {
> > > +           dev_err(dev, "failed to init entity pads: %d", ret);
> > > +           goto err_free_handler;
> > > +   }
> > > +
> > > +   pm_runtime_enable(dev);
> > > +   if (!pm_runtime_enabled(dev)) {
> > > +           ret = ov02a10_power_on(dev);
> > > +           if (ret < 0) {
> > > +                   dev_err(dev, "failed to power on: %d\n", ret);
> > > +                   goto err_free_handler;
> > > +           }
> > > +   }
> > > +
> > > +   ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > > +   if (ret) {
> > > +           dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > > +           if (!pm_runtime_enabled(dev))
> > > +                   ov02a10_power_off(dev);
>
> This should be moved to error handling section below.
>
> > > +           goto err_clean_entity;
> > > +   }
> >
> > Tomasz, Sakari, is this ok?
> > or coding like this:
> >
> > ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > if (!pm_runtime_enabled(dev))
> >       ov02a10_power_off(dev);
> > if (ret) {
> >       dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> >       goto err_clean_entity;
> > }
> >
> > What's your opinions about the change?
>
> This turns power off if runtime PM is disabled. I'd keep it as-is, as it'd
> require re-implementing what runtime PM is used for now --- and that's not
> a sensor driver's job.

That and in general I believe the expectations are:

- runtime PM enabled - powered on only when it has something to do
- runtime PM disabled - powered on when the driver is bound (probe
succeeded), powered off when the driver unbinds (remove or probe
error)

Best regards,
Tomasz

>
> >
> > > +
> > > +   return 0;
> > > +
> > > +err_clean_entity:
> > > +   media_entity_cleanup(&ov02a10->subdev.entity);
> > > +err_free_handler:
> > > +   v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > > +err_destroy_mutex:
> > > +   mutex_destroy(&ov02a10->mutex);
> > > +
> > > +   return ret;
> > > +}
> > > +
> > > +static int ov02a10_remove(struct i2c_client *client)
> > > +{
> > > +   struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > +   struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > > +
> > > +   v4l2_async_unregister_subdev(sd);
> > > +   media_entity_cleanup(&sd->entity);
> > > +   v4l2_ctrl_handler_free(sd->ctrl_handler);
> > > +   pm_runtime_disable(&client->dev);
> > > +   if (!pm_runtime_status_suspended(&client->dev))
> > > +           ov02a10_power_off(&client->dev);
> > > +   pm_runtime_set_suspended(&client->dev);
> > > +   mutex_destroy(&ov02a10->mutex);
> > > +
> > > +   return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ov02a10_of_match[] = {
> > > +   { .compatible = "ovti,ov02a10" },
> > > +   {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > > +
> > > +static struct i2c_driver ov02a10_i2c_driver = {
> > > +   .driver = {
> > > +           .name = "ov02a10",
> > > +           .pm = &ov02a10_pm_ops,
> > > +           .of_match_table = ov02a10_of_match,
> > > +   },
> > > +   .probe_new      = &ov02a10_probe,
> > > +   .remove         = &ov02a10_remove,
> > > +};
> > > +
> > > +module_i2c_driver(ov02a10_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> >
>
> --
> Sakari Ailus

^ permalink raw reply

* Re: [PATCH V2 1/2] mmc: sdhci-msm: Add interconnect bandwidth scaling support
From: Sibi Sankar @ 2020-06-04 18:34 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Pradeep P V K, bjorn.andersson, adrian.hunter, robh+dt,
	ulf.hansson, vbadigan, sboyd, georgi.djakov, linux-mmc,
	linux-kernel, linux-arm-msm, devicetree, linux-mmc-owner, rnayak,
	matthias
In-Reply-To: <20200604170906.GP4525@google.com>

On 2020-06-04 22:39, Matthias Kaehlcke wrote:
> On Thu, Jun 04, 2020 at 04:44:42PM +0530, Pradeep P V K wrote:
>> Interconnect bandwidth scaling support is now added as a
>> part of OPP [1]. So, make sure interconnect driver is ready
>> before handling interconnect scaling.
>> 
>> This change is based on
>> [1] [Patch v8] Introduce OPP bandwidth bindings
>> (https://lkml.org/lkml/2020/5/12/493)
>> 
>> [2] [Patch v3] mmc: sdhci-msm: Fix error handling
>> for dev_pm_opp_of_add_table()
>> (https://lkml.org/lkml/2020/5/5/491)
>> 
>> Signed-off-by: Pradeep P V K <ppvk@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/drivers/mmc/host/sdhci-msm.c 
>> b/drivers/mmc/host/sdhci-msm.c
>> index b277dd7..a13ff1b 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -14,6 +14,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/iopoll.h>
>>  #include <linux/regulator/consumer.h>
>> +#include <linux/interconnect.h>
>> 
>>  #include "sdhci-pltfm.h"
>>  #include "cqhci.h"
>> @@ -2070,6 +2071,18 @@ static int sdhci_msm_probe(struct 
>> platform_device *pdev)
>>  	}
>>  	msm_host->bulk_clks[0].clk = clk;
>> 
>> +	/* Make sure that ICC driver is ready for interconnect bandwdith
>> +	 * scaling before registering the device for OPP.
>> +	 */
>> +	ret = dev_pm_opp_of_find_icc_paths(&pdev->dev, NULL);
>> +	if (ret) {
>> +		if (ret == -EPROBE_DEFER)
>> +			dev_info(&pdev->dev, "defer icc path: %d\n", ret);
> 
> I already commented on this on v1:
> 
>   This log seems to add little more than noise, or are there particular 
> reasons
>   why it is useful in this driver? Most drivers just return silently in 
> case of
>   deferred probing.
> 
> If you think the log is really needed please explain why.

Both the err logs seem redundant.
EPROBE_DEFERS are rather readily
noticeable through the return val.
dev_.._find_icc_paths already prints
err messages when we fail to get icc
paths.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

^ permalink raw reply

* Re: [v2] drm/bridge: ti-sn65dsi86: ensure bridge suspend happens during PM sleep
From: Sam Ravnborg @ 2020-06-04 19:33 UTC (permalink / raw)
  To: Harigovindan P
  Cc: dri-devel, linux-arm-msm, freedreno, devicetree, linux-kernel,
	robdclark, seanpaul, hoegsberg, kalyan_t, nganji
In-Reply-To: <20200604103438.23667-1-harigovi@codeaurora.org>

Hi Harigovindan
On Thu, Jun 04, 2020 at 04:04:38PM +0530, Harigovindan P wrote:
> ti-sn65dsi86 bridge is enumerated as a runtime device.
> 
> Adding sleep ops to force runtime_suspend when PM suspend is
> requested on the device.

Patch looks correct - but could you please explain why it is needed.
What is the gain compared to not having this patch.

I ask for two reasons:
1) I really do not know
2) this info should be in the changelog

Without a better changelog no ack from me - sorry.

	Sam

> 
> Signed-off-by: Harigovindan P <harigovi@codeaurora.org>
> ---
> Changes in v2:
> 	- Include bridge name in the commit message and 
> 	remove dependent patchwork link from the commit
> 	text as bridge is independent of OEM(Stephen Boyd)
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 6ad688b320ae..2eef755b2917 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -159,6 +159,8 @@ static int __maybe_unused ti_sn_bridge_suspend(struct device *dev)
>  
>  static const struct dev_pm_ops ti_sn_bridge_pm_ops = {
>  	SET_RUNTIME_PM_OPS(ti_sn_bridge_suspend, ti_sn_bridge_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
>  };
>  
>  static int status_show(struct seq_file *s, void *data)
> -- 
> 2.27.0

^ permalink raw reply

* Re: [PATCH v8 2/6] ARM: tegra: Add device-tree for ASUS Google Nexus 7
From: Dmitry Osipenko @ 2020-06-04 20:11 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Rob Herring, Thierry Reding, Jonathan Hunter, David Heidelberg,
	Peter Geis, Stephen Warren, Nicolas Chauvet, Pedro Ângelo,
	Matt Merhar, Zack Pearsall, linux-tegra, devicetree, linux-kernel
In-Reply-To: <cf73df1a-92ed-f509-74e8-1c847945fb14@gmail.com>

16.05.2020 15:01, Dmitry Osipenko пишет:
> 15.05.2020 21:18, Michał Mirosław пишет:
>> On Fri, May 15, 2020 at 12:36:50AM +0300, Dmitry Osipenko wrote:
>>> There are few hardware variants of NVIDIA Tegra30-based Nexus 7 device:
>>>
>>> 1. WiFi-only (named Grouper)
>>> 2. GSM (named Tilapia)
>>> 3. Using Maxim PMIC (E1565 board ID)
>>> 4. Using Ti PMIC (PM269 board ID)
>>
>> Hi,
>>
>> I've briefly looked at the PM269 devicetree (PMIC part) and it looks very
>> similar, if not the same, to what I deduced from the TF300T kernel.
> 
> Hello Michał,
> 
> Definitely there are board parts that are reused by different devices.
> This is not surprising since most of the boards are designed by the same
> company.
> 
>> Those devices don't look to differ much from original Cardhu tablet
>> devkit, so maybe the trees can base off of that?
> 
> I don't think it's really possible in a case of Nexus 7 because in
> overall the used hardware components differ a bit too much. It shouldn't
> worth the effort, IMO.
> 
>> I would also guess that because of this 'ram-code', memory timings would
>> be duplicated between devices. I can see small differences between
>> ram-code=1 timings of Grouper and TF300T, though they look like arbiter
>> tuning differences. I'll have to test if my TF300T works with Grouper's
>> settings. In case they work, could you split the memory timings to another
>> dtsi file?
> 
> Yes, perhaps this could be done. The memory timings on Grouper and
> Tilapia are pretty much the same as well. As you noticed, there are some
> tuning differences of TF300T in comparison to the Nexus 7, the same
> applies to the Grouper and Tilapia variants.
> 
>> BTW, shouldn't EMC timing set match MC? I see more frequencies listed in
>> MC than EMC nodes.
> 
> The MC timings are exactly the same on Grouper and Tilapia, but EMC
> timings have a very minor differences, and thus, the common.dtsi misses
> these differentiating EMC timings, they are defined in grouper.dtsi and
> tilapia.dts.
> 
> I guess we indeed could try to select the lowest common denominator
> timing and re-use it. I'll consider this change for v9, thank you very
> much for the suggestion.

Hello Michał,

I tried to investigate whether we could unify the memory timings and
it's hard to tell. The values are mostly off by one in most cases for
one or two registers, in others cases a memory-chip feature is enabled
or disabled for the same memory module. It certainly feels like we could
safely ignore all those minor differences, but I don't have enough
expertise in order to decide firmly. In my opinion it should be better
to leave the memory timings as-is for the starter, we could always get
back to the unification later on.

^ permalink raw reply

* Re: [PATCH 3/3] spi: bcm2835: Enable shared interrupt support
From: Florian Fainelli @ 2020-06-04 20:24 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-kernel, Rob Herring, Nicolas Saenz Julienne, Ray Jui,
	Scott Branden,
	maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE...,
	open list:SPI SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
	Martin Sperl, lukas
In-Reply-To: <21772111-fa1f-7a50-aa92-e44b09cff4eb@gmail.com>



On 6/4/2020 9:05 AM, Florian Fainelli wrote:
> 
> 
> On 6/4/2020 5:32 AM, Mark Brown wrote:
>> On Wed, Jun 03, 2020 at 08:46:55PM -0700, Florian Fainelli wrote:
>>> The SPI controller found in the BCM2711 and BCM7211 SoCs is instantiated
>>> 5 times, with all instances sharing the same interrupt line. We
>>> specifically match the two compatible strings here to determine whether
>>> it is necessary to request the interrupt with the IRQF_SHARED flag and
>>> to use an appropriate interrupt handler capable of returning IRQ_NONE.
>>
>>> For the BCM2835 case which is deemed performance critical, there is no
>>> overhead since a dedicated handler that does not assume sharing is used.
>>
>> This feels hacky - it's essentially using the compatible string to set a
>> boolean flag which isn't really about the IP but rather the platform
>> integration.  It might cause problems if we do end up having to quirk
>> this version of the IP for some other reason.
> 
> I am not sure why it would be a problem, when you describe a piece of
> hardware with Device Tree, even with the IP block being strictly the
> same, its very integration into a new SoC (with details like shared
> interrupt lines) do warrant a different compatible string. Maybe this is
> more of a philosophical question.
> 
>> I'm also looking at the
>> code and wondering if the overhead of checking to see if the interrupt
>> is flagged is really that severe, it's just a check to see if a bit is
>> set in a register which we already read so should be a couple of
>> instructions (which disassembly seems to confirm).  It *is* overhead so
>> there's some value in it, I'm just surprised that it's such a hot path
>> especially with a reasonably deep FIFO like this device has.
> 
> If it was up to me, we would just add the check on BCM2835_SPI_CS_INTR
> not being set and return IRQ_NONE and be done with it. I appreciate that
> Lukas has spent some tremendous amount of time working on this
> controller driver and he has a sensitivity for performance.
> 
>>
>> I guess ideally genirq would provide a way to figure out if an interrupt
>> is actually shared in the present system, and better yet we'd have a way
>> for drivers to say they aren't using the interrupt ATM, but that might
>> be more effort than it's really worth.  If this is needed and there's no
>> better way of figuring out if the interrupt is really shared then I'd
>> suggest a boolean flag rather than a compatible string, it's still a
>> hack but it's less likely to store up trouble for the future.
> 
> Instead of counting the number of SPI devices we culd request the
> interrupt first with flags = IRQF_PROBE_SHARED, if this works, good we
> have a single SPI master enabled, if it returns -EBUSY, try again with
> flags = IRQF_SHARED and set-up the bcm2835_spi_sh_interrupt interrupt
> handler to manage the sharing.
> 
> This would not require DT changes, which is probably better anyway.
Unfortunately this does not work.. The first time we probe the driver we
need to set an interrupt handler that is capable of handling a shared
interrupt. When we probe for subsequent times, we can use the -EBUSY
return code to know that we are in a shared interrupt context, however,
it is too late to change the first controller interrupt handler.

So we do need to know for the first time we install the interrupt
handler whether we will be in a shared situation or not, I cannot think
of any solution other than counting the number of available DT nodes
with the "brcm,bcm2835-spi" compatible string.
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next v6 4/4] net: dp83869: Add RGMII internal delay configuration
From: Dan Murphy @ 2020-06-04 20:27 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew, f.fainelli, hkallweit1, davem, robh, netdev, linux-kernel,
	devicetree
In-Reply-To: <20200604094829.0d7d5df7@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

Jakub

On 6/4/20 11:48 AM, Jakub Kicinski wrote:
> On Thu, 4 Jun 2020 11:38:14 -0500 Dan Murphy wrote:
>> Jakub
>>
>> On 6/4/20 11:25 AM, Jakub Kicinski wrote:
>>> On Thu, 4 Jun 2020 06:14:10 -0500 Dan Murphy wrote:
>>>> Add RGMII internal delay configuration for Rx and Tx.
>>>>
>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>> Hi Dan, please make sure W=1 C=1 build is clean:
>>>
>>> drivers/net/phy/dp83869.c:103:18: warning: ‘dp83869_internal_delay’ defined but not used [-Wunused-const-variable=]
>>>     103 | static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
>>>         |                  ^~~~~~~~~~~~~~~~~~~~~~
>> I built with W=1 and C=1 and did not see this warning.
>>
>> What defconfig are you using?
> allmodconfig with gcc-10
>
>> Can you check if CONFIG_OF_MDIO is set or not?  That would be the only
>> way that warning would come up.
> Hm. I don't have the config from this particular build but just running
> allmodconfig makes it CONFIG_OF_MDIO=m

OK that makes sense then.  That is an existing bug that shows up because 
of this.

#ifdef CONFIG_OF_MDIO

So the addition of the array exposed an existing issue.

That bug fix can go to net then.
>>> Also net-next is closed right now, you can post RFCs but normal patches
>>> should be deferred until after net-next reopens.
>> I know net-next is closed.
>>
>> I pinged David M when it was open about what is meant by "new" patches
>> in the net-dev FAQ.  So I figured I would send the patches to see what
>> the response was.
>>
>> To me these are not new they are in process patches.  My understand is
>> New is v1 patchesets.
>>
>> But now I have the answer.
> Oh sorry, I may be wrong in this case, I haven't tracked this series.
>
It says v6 in $subject.

But still you may be correct I don't know

Dan


^ permalink raw reply

* Re: [PATCH v6] drm/msm/dpu: ensure device suspend happens during PM sleep
From: Doug Anderson @ 2020-06-04 20:34 UTC (permalink / raw)
  To: Kalyan Thota
  Cc: dri-devel, linux-arm-msm, freedreno,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
	Rob Clark, Sean Paul, Kristian H. Kristensen, Jeykumar Sankaran,
	mkrishn, travitej, nganji
In-Reply-To: <1591276775-13949-1-git-send-email-kalyan_t@codeaurora.org>

Hi,

On Thu, Jun 4, 2020 at 6:20 AM Kalyan Thota <kalyan_t@codeaurora.org> wrote:
>
> -#ifdef CONFIG_PM
> -static int msm_runtime_suspend(struct device *dev)
> +#ifdef CONFIG_PM_SLEEP
> +static int msm_pm_suspend(struct device *dev)
>  {
> -       struct drm_device *ddev = dev_get_drvdata(dev);
> -       struct msm_drm_private *priv = ddev->dev_private;
> -       struct msm_mdss *mdss = priv->mdss;
>

nit: remove blank line at the start of this function

>  static const struct dev_pm_ops msm_pm_ops = {
>         SET_SYSTEM_SLEEP_PM_OPS(msm_pm_suspend, msm_pm_resume)
>         SET_RUNTIME_PM_OPS(msm_runtime_suspend, msm_runtime_resume, NULL)
> +       .prepare = msm_pm_prepare,
> +       .complete = msm_pm_complete,

Presumably you will get a compile failure if someone compiles without
CONFIG_PM_SLEEP since msm_pm_prepare() and msm_pm_complete() won't be
defined but you refer to them unconditionally.  Probably the best
solution is to just add "__maybe_unused" to your prepare/complete
function and then always define them.


I can't say I've thought through every corner case but at least this
change no longer raises alarm bells in my mind when I look at it.  ;-)
 If it works for you and nobody else has objections then it seems good
enough and we can always make more improvements later.  Feel free to
add my Reviewed-by tag when my nit is fixed and you make sure it
compiles even if CONFIG_PM_SLEEP isn't defined.

-Doug

^ permalink raw reply

* Re: [PATCH 1/5] RISC-V: Add mechanism to provide custom IPI operations
From: Palmer Dabbelt @ 2020-06-04 20:40 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, aou, robh+dt, daniel.lezcano, tglx, Damien Le Moal,
	Atish Patra, Alistair Francis, anup, linux-riscv, linux-kernel,
	devicetree, Anup Patel
In-Reply-To: <20200521134544.816918-2-anup.patel@wdc.com>

On Thu, 21 May 2020 06:45:40 PDT (-0700), Anup Patel wrote:
> We add mechanism to set custom IPI operations so that CLINT driver
> from drivers directory can provide custom IPI operations.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  arch/riscv/include/asm/smp.h | 11 ++++++++
>  arch/riscv/kernel/smp.c      | 52 ++++++++++++++++++++++++------------
>  arch/riscv/kernel/smpboot.c  |  3 +--
>  3 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
> index 40bb1c15a731..ad0601260cb1 100644
> --- a/arch/riscv/include/asm/smp.h
> +++ b/arch/riscv/include/asm/smp.h
> @@ -40,6 +40,17 @@ void arch_send_call_function_single_ipi(int cpu);
>  int riscv_hartid_to_cpuid(int hartid);
>  void riscv_cpuid_to_hartid_mask(const struct cpumask *in, struct cpumask *out);
>
> +struct riscv_ipi_ops {
> +	void (*ipi_inject)(const unsigned long *hart_mask);
> +	void (*ipi_clear)(void);
> +};
> +
> +/* Set custom IPI operations */
> +void riscv_set_ipi_ops(struct riscv_ipi_ops *ops);
> +
> +/* Clear IPI for current CPU */
> +void riscv_clear_ipi(void);
> +
>  /*
>   * Obtains the hart ID of the currently executing task.  This relies on
>   * THREAD_INFO_IN_TASK, but we define that unconditionally.
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index b1d4f452f843..8375cc5970f6 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -84,6 +84,35 @@ static void ipi_stop(void)
>  		wait_for_interrupt();
>  }
>
> +#if IS_ENABLED(CONFIG_RISCV_SBI)
> +static void clear_ipi(void)
> +{
> +	csr_clear(CSR_IP, IE_SIE);
> +}
> +
> +static struct riscv_ipi_ops sbi_ipi_ops = {
> +	.ipi_inject = sbi_send_ipi,
> +	.ipi_clear = clear_ipi,
> +};
> +
> +static struct riscv_ipi_ops *ipi_ops = &sbi_ipi_ops;
> +#else
> +static struct riscv_ipi_ops *ipi_ops;
> +#endif
> +
> +void riscv_set_ipi_ops(struct riscv_ipi_ops *ops)
> +{
> +	ipi_ops = ops;
> +}
> +EXPORT_SYMBOL_GPL(riscv_set_ipi_ops);
> +
> +void riscv_clear_ipi(void)
> +{
> +	if (ipi_ops)
> +		ipi_ops->ipi_clear();
> +}
> +EXPORT_SYMBOL_GPL(riscv_clear_ipi);

There should at least be a warning on SMP systems when an ipi_ops hasn't been
set, as otherwise the system will just hang.

> +
>  static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
>  {
>  	struct cpumask hartid_mask;
> @@ -95,10 +124,9 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op)
>  	smp_mb__after_atomic();
>
>  	riscv_cpuid_to_hartid_mask(mask, &hartid_mask);
> -	if (IS_ENABLED(CONFIG_RISCV_SBI))
> -		sbi_send_ipi(cpumask_bits(&hartid_mask));
> -	else
> -		clint_send_ipi_mask(mask);
> +
> +	if (ipi_ops)
> +		ipi_ops->ipi_inject(cpumask_bits(&hartid_mask));
>  }
>
>  static void send_ipi_single(int cpu, enum ipi_message_type op)
> @@ -109,18 +137,8 @@ static void send_ipi_single(int cpu, enum ipi_message_type op)
>  	set_bit(op, &ipi_data[cpu].bits);
>  	smp_mb__after_atomic();
>
> -	if (IS_ENABLED(CONFIG_RISCV_SBI))
> -		sbi_send_ipi(cpumask_bits(cpumask_of(hartid)));
> -	else
> -		clint_send_ipi_single(hartid);
> -}
> -
> -static inline void clear_ipi(void)
> -{
> -	if (IS_ENABLED(CONFIG_RISCV_SBI))
> -		csr_clear(CSR_IP, IE_SIE);
> -	else
> -		clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> +	if (ipi_ops)
> +		ipi_ops->ipi_inject(cpumask_bits(cpumask_of(hartid)));
>  }
>
>  void handle_IPI(struct pt_regs *regs)
> @@ -131,7 +149,7 @@ void handle_IPI(struct pt_regs *regs)
>
>  	irq_enter();
>
> -	clear_ipi();
> +	riscv_clear_ipi();
>
>  	while (true) {
>  		unsigned long ops;
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 4e9922790f6e..5fe849791bf0 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -147,8 +147,7 @@ asmlinkage __visible void smp_callin(void)
>  {
>  	struct mm_struct *mm = &init_mm;
>
> -	if (!IS_ENABLED(CONFIG_RISCV_SBI))
> -		clint_clear_ipi(cpuid_to_hartid_map(smp_processor_id()));
> +	riscv_clear_ipi();
>
>  	/* All kernel threads share the same mm context.  */
>  	mmgrab(mm);

^ permalink raw reply

* Re: [PATCH 2/5] RISC-V: Remove CLINT related code
From: Palmer Dabbelt @ 2020-06-04 20:40 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, aou, robh+dt, daniel.lezcano, tglx, Damien Le Moal,
	Atish Patra, Alistair Francis, anup, linux-riscv, linux-kernel,
	devicetree, Anup Patel
In-Reply-To: <20200521134544.816918-3-anup.patel@wdc.com>

On Thu, 21 May 2020 06:45:41 PDT (-0700), Anup Patel wrote:
> We will be having separate CLINT timer driver which will also
> provide CLINT based IPI operations so let's remove CLINT related
> code from arch/riscv directory.

This will leave the system unbootable, which breaks bisecting.

>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  arch/riscv/include/asm/clint.h | 39 ------------------------------
>  arch/riscv/kernel/Makefile     |  2 +-
>  arch/riscv/kernel/clint.c      | 44 ----------------------------------
>  arch/riscv/kernel/setup.c      |  2 --
>  arch/riscv/kernel/smp.c        |  1 -
>  arch/riscv/kernel/smpboot.c    |  1 -
>  6 files changed, 1 insertion(+), 88 deletions(-)
>  delete mode 100644 arch/riscv/include/asm/clint.h
>  delete mode 100644 arch/riscv/kernel/clint.c
>
> diff --git a/arch/riscv/include/asm/clint.h b/arch/riscv/include/asm/clint.h
> deleted file mode 100644
> index a279b17a6aad..000000000000
> --- a/arch/riscv/include/asm/clint.h
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef _ASM_RISCV_CLINT_H
> -#define _ASM_RISCV_CLINT_H 1
> -
> -#include <linux/io.h>
> -#include <linux/smp.h>
> -
> -#ifdef CONFIG_RISCV_M_MODE
> -extern u32 __iomem *clint_ipi_base;
> -
> -void clint_init_boot_cpu(void);
> -
> -static inline void clint_send_ipi_single(unsigned long hartid)
> -{
> -	writel(1, clint_ipi_base + hartid);
> -}
> -
> -static inline void clint_send_ipi_mask(const struct cpumask *mask)
> -{
> -	int cpu;
> -
> -	for_each_cpu(cpu, mask)
> -		clint_send_ipi_single(cpuid_to_hartid_map(cpu));
> -}
> -
> -static inline void clint_clear_ipi(unsigned long hartid)
> -{
> -	writel(0, clint_ipi_base + hartid);
> -}
> -#else /* CONFIG_RISCV_M_MODE */
> -#define clint_init_boot_cpu()	do { } while (0)
> -
> -/* stubs to for code is only reachable under IS_ENABLED(CONFIG_RISCV_M_MODE): */
> -void clint_send_ipi_single(unsigned long hartid);
> -void clint_send_ipi_mask(const struct cpumask *hartid_mask);
> -void clint_clear_ipi(unsigned long hartid);
> -#endif /* CONFIG_RISCV_M_MODE */
> -
> -#endif /* _ASM_RISCV_CLINT_H */
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index d8bbd3207100..529cda705cfe 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -31,7 +31,7 @@ obj-y	+= cacheinfo.o
>  obj-y	+= patch.o
>  obj-$(CONFIG_MMU) += vdso.o vdso/
>
> -obj-$(CONFIG_RISCV_M_MODE)	+= clint.o traps_misaligned.o
> +obj-$(CONFIG_RISCV_M_MODE)	+= traps_misaligned.o
>  obj-$(CONFIG_FPU)		+= fpu.o
>  obj-$(CONFIG_SMP)		+= smpboot.o
>  obj-$(CONFIG_SMP)		+= smp.o
> diff --git a/arch/riscv/kernel/clint.c b/arch/riscv/kernel/clint.c
> deleted file mode 100644
> index 3647980d14c3..000000000000
> --- a/arch/riscv/kernel/clint.c
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -/*
> - * Copyright (c) 2019 Christoph Hellwig.
> - */
> -
> -#include <linux/io.h>
> -#include <linux/of_address.h>
> -#include <linux/types.h>
> -#include <asm/clint.h>
> -#include <asm/csr.h>
> -#include <asm/timex.h>
> -#include <asm/smp.h>
> -
> -/*
> - * This is the layout used by the SiFive clint, which is also shared by the qemu
> - * virt platform, and the Kendryte KD210 at least.
> - */
> -#define CLINT_IPI_OFF		0
> -#define CLINT_TIME_CMP_OFF	0x4000
> -#define CLINT_TIME_VAL_OFF	0xbff8
> -
> -u32 __iomem *clint_ipi_base;
> -
> -void clint_init_boot_cpu(void)
> -{
> -	struct device_node *np;
> -	void __iomem *base;
> -
> -	np = of_find_compatible_node(NULL, NULL, "riscv,clint0");
> -	if (!np) {
> -		panic("clint not found");
> -		return;
> -	}
> -
> -	base = of_iomap(np, 0);
> -	if (!base)
> -		panic("could not map CLINT");
> -
> -	clint_ipi_base = base + CLINT_IPI_OFF;
> -	riscv_time_cmp = base + CLINT_TIME_CMP_OFF;
> -	riscv_time_val = base + CLINT_TIME_VAL_OFF;
> -
> -	clint_clear_ipi(boot_cpu_hartid);
> -}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 145128a7e560..b07a583bf53b 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -18,7 +18,6 @@
>  #include <linux/swiotlb.h>
>  #include <linux/smp.h>
>
> -#include <asm/clint.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/setup.h>
>  #include <asm/sections.h>
> @@ -76,7 +75,6 @@ void __init setup_arch(char **cmdline_p)
>  	setup_bootmem();
>  	paging_init();
>  	unflatten_device_tree();
> -	clint_init_boot_cpu();
>
>  #ifdef CONFIG_SWIOTLB
>  	swiotlb_init(1);
> diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
> index 8375cc5970f6..8a23f1eb5400 100644
> --- a/arch/riscv/kernel/smp.c
> +++ b/arch/riscv/kernel/smp.c
> @@ -17,7 +17,6 @@
>  #include <linux/seq_file.h>
>  #include <linux/delay.h>
>
> -#include <asm/clint.h>
>  #include <asm/sbi.h>
>  #include <asm/tlbflush.h>
>  #include <asm/cacheflush.h>
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 5fe849791bf0..a6cfa9842d4b 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -24,7 +24,6 @@
>  #include <linux/of.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/sched/mm.h>
> -#include <asm/clint.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/irq.h>
>  #include <asm/mmu_context.h>

^ permalink raw reply

* Re: [PATCH 3/5] clocksource/drivers/timer-riscv: Remove MMIO related stuff
From: Palmer Dabbelt @ 2020-06-04 20:40 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, aou, robh+dt, daniel.lezcano, tglx, Damien Le Moal,
	Atish Patra, Alistair Francis, anup, linux-riscv, linux-kernel,
	devicetree, Anup Patel
In-Reply-To: <20200521134544.816918-4-anup.patel@wdc.com>

On Thu, 21 May 2020 06:45:42 PDT (-0700), Anup Patel wrote:
> Right now the RISC-V timer is convoluted to support:
> 1. Linux RISC-V S-mode (with MMU) where it will use TIME CSR
>    for clocksource and SBI timer calls for clockevent device.
> 2. Linux RISC-V M-mode (without MMU) where it will use CLINT
>    MMIO counter register for clocksource and CLINT MMIO compare
>    register for clockevent device.
>
> This patch removes MMIO related stuff from RISC-V timer driver
> so that we can have a separate CLINT timer driver.

This one will also break bisecting for the K210.

>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  arch/riscv/Kconfig                |  2 +-
>  arch/riscv/include/asm/timex.h    | 28 +++++++---------------------
>  drivers/clocksource/Kconfig       |  2 +-
>  drivers/clocksource/timer-riscv.c | 17 ++---------------
>  4 files changed, 11 insertions(+), 38 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 2cf0c83c1a47..bbdc37a78f7b 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -52,7 +52,7 @@ config RISCV
>  	select PCI_DOMAINS_GENERIC if PCI
>  	select PCI_MSI if PCI
>  	select RISCV_INTC
> -	select RISCV_TIMER
> +	select RISCV_TIMER if RISCV_SBI
>  	select GENERIC_IRQ_MULTI_HANDLER
>  	select GENERIC_ARCH_TOPOLOGY if SMP
>  	select ARCH_HAS_PTE_SPECIAL
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index bad2a7c2cda5..a3fb85d505d4 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -7,41 +7,27 @@
>  #define _ASM_RISCV_TIMEX_H
>
>  #include <asm/csr.h>
> -#include <asm/mmio.h>
>
>  typedef unsigned long cycles_t;
>
> -extern u64 __iomem *riscv_time_val;
> -extern u64 __iomem *riscv_time_cmp;
> -
> -#ifdef CONFIG_64BIT
> -#define mmio_get_cycles()	readq_relaxed(riscv_time_val)
> -#else
> -#define mmio_get_cycles()	readl_relaxed(riscv_time_val)
> -#define mmio_get_cycles_hi()	readl_relaxed(((u32 *)riscv_time_val) + 1)
> -#endif
> -
>  static inline cycles_t get_cycles(void)
>  {
> -	if (IS_ENABLED(CONFIG_RISCV_SBI))
> -		return csr_read(CSR_TIME);
> -	return mmio_get_cycles();
> +	return csr_read(CSR_TIME);
>  }
>  #define get_cycles get_cycles
>
> +static inline u32 get_cycles_hi(void)
> +{
> +	return csr_read(CSR_TIMEH);
> +}
> +#define get_cycles_hi get_cycles_hi
> +
>  #ifdef CONFIG_64BIT
>  static inline u64 get_cycles64(void)
>  {
>  	return get_cycles();
>  }
>  #else /* CONFIG_64BIT */
> -static inline u32 get_cycles_hi(void)
> -{
> -	if (IS_ENABLED(CONFIG_RISCV_SBI))
> -		return csr_read(CSR_TIMEH);
> -	return mmio_get_cycles_hi();
> -}
> -
>  static inline u64 get_cycles64(void)
>  {
>  	u32 hi, lo;
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index f2142e6bbea3..21950d9e3e9d 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -650,7 +650,7 @@ config ATCPIT100_TIMER
>
>  config RISCV_TIMER
>  	bool "Timer for the RISC-V platform"
> -	depends on GENERIC_SCHED_CLOCK && RISCV
> +	depends on GENERIC_SCHED_CLOCK && RISCV_SBI
>  	default y
>  	select TIMER_PROBE
>  	select TIMER_OF
> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
> index 5fb7c5ba5c91..3e7e0cf5b899 100644
> --- a/drivers/clocksource/timer-riscv.c
> +++ b/drivers/clocksource/timer-riscv.c
> @@ -19,26 +19,13 @@
>  #include <linux/of_irq.h>
>  #include <asm/smp.h>
>  #include <asm/sbi.h>
> -
> -u64 __iomem *riscv_time_cmp;
> -u64 __iomem *riscv_time_val;
> -
> -static inline void mmio_set_timer(u64 val)
> -{
> -	void __iomem *r;
> -
> -	r = riscv_time_cmp + cpuid_to_hartid_map(smp_processor_id());
> -	writeq_relaxed(val, r);
> -}
> +#include <asm/timex.h>
>
>  static int riscv_clock_next_event(unsigned long delta,
>  		struct clock_event_device *ce)
>  {
>  	csr_set(CSR_IE, IE_TIE);
> -	if (IS_ENABLED(CONFIG_RISCV_SBI))
> -		sbi_set_timer(get_cycles64() + delta);
> -	else
> -		mmio_set_timer(get_cycles64() + delta);
> +	sbi_set_timer(get_cycles64() + delta);
>  	return 0;
>  }

^ permalink raw reply

* Re: [PATCH 4/5] clocksource/drivers: Add CLINT timer driver
From: Palmer Dabbelt @ 2020-06-04 20:40 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, aou, robh+dt, daniel.lezcano, tglx, Damien Le Moal,
	Atish Patra, Alistair Francis, anup, linux-riscv, linux-kernel,
	devicetree, Anup Patel
In-Reply-To: <20200521134544.816918-5-anup.patel@wdc.com>

On Thu, 21 May 2020 06:45:43 PDT (-0700), Anup Patel wrote:
> The TIME CSR and SBI calls are not available in RISC-V M-mode so we
> add CLINT driver for Linux RISC-V M-mode (i.e. RISC-V NoMMU kernel).
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  drivers/clocksource/Kconfig       |  10 ++
>  drivers/clocksource/Makefile      |   1 +
>  drivers/clocksource/timer-clint.c | 226 ++++++++++++++++++++++++++++++
>  include/linux/cpuhotplug.h        |   1 +
>  4 files changed, 238 insertions(+)
>  create mode 100644 drivers/clocksource/timer-clint.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 21950d9e3e9d..ea97bf0eb09f 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -659,6 +659,16 @@ config RISCV_TIMER
>  	  is accessed via both the SBI and the rdcycle instruction.  This is
>  	  required for all RISC-V systems.
>
> +config CLINT_TIMER
> +	bool "Timer for the RISC-V platform"
> +	depends on GENERIC_SCHED_CLOCK && RISCV

Presumably this also depends on RISCV_M_MODE?

> +	default y
> +	select TIMER_PROBE
> +	select TIMER_OF
> +	help
> +	  This option enables the CLINT timer for RISC-V systems. The CLINT
> +	  driver is usually used for NoMMU RISC-V systems.
> +
>  config CSKY_MP_TIMER
>  	bool "SMP Timer for the C-SKY platform" if COMPILE_TEST
>  	depends on CSKY
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 641ba5383ab5..dca308b5ff98 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -86,6 +86,7 @@ obj-$(CONFIG_CLKSRC_ST_LPC)		+= clksrc_st_lpc.o
>  obj-$(CONFIG_X86_NUMACHIP)		+= numachip.o
>  obj-$(CONFIG_ATCPIT100_TIMER)		+= timer-atcpit100.o
>  obj-$(CONFIG_RISCV_TIMER)		+= timer-riscv.o
> +obj-$(CONFIG_CLINT_TIMER)		+= timer-clint.o
>  obj-$(CONFIG_CSKY_MP_TIMER)		+= timer-mp-csky.o
>  obj-$(CONFIG_GX6605S_TIMER)		+= timer-gx6605s.o
>  obj-$(CONFIG_HYPERV_TIMER)		+= hyperv_timer.o
> diff --git a/drivers/clocksource/timer-clint.c b/drivers/clocksource/timer-clint.c
> new file mode 100644
> index 000000000000..7fc4f145da65
> --- /dev/null
> +++ b/drivers/clocksource/timer-clint.c
> @@ -0,0 +1,226 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> + *
> + * Most of the M-mode (i.e. NoMMU) RISC-V systems usually have a
> + * CLINT MMIO timer device.
> + */
> +
> +#define pr_fmt(fmt) "clint: " fmt
> +#include <linux/bitops.h>
> +#include <linux/clocksource.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/sched_clock.h>
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +#include <linux/irqchip/irq-riscv-intc.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_irq.h>
> +#include <linux/smp.h>
> +
> +#define CLINT_IPI_OFF		0
> +#define CLINT_TIME_CMP_OFF	0x4000
> +#define CLINT_TIME_VAL_OFF	0xbff8
> +
> +/* CLINT manages IPI and Timer for RISC-V M-mode  */
> +static u32 __iomem *clint_ipi_base;

It seems odd to have IPIs in the timer driver.  I know the CLINT handles both
of them, but these really feel like two separate drivers.

> +static u64 __iomem *clint_time_cmp;
> +static u64 __iomem *clint_time_val;
> +static unsigned long clint_freq;
> +static unsigned int clint_irq;
> +
> +static void clint_send_ipi(const unsigned long *hart_mask)
> +{
> +	u32 hartid;
> +
> +	for_each_set_bit(hartid, hart_mask, NR_CPUS)
> +		writel(1, clint_ipi_base + hartid);
> +}
> +
> +static void clint_clear_ipi(void)
> +{
> +	writel(0, clint_ipi_base + cpuid_to_hartid_map(smp_processor_id()));
> +}
> +
> +static struct riscv_ipi_ops clint_ipi_ops = {
> +	.ipi_inject = clint_send_ipi,
> +	.ipi_clear = clint_clear_ipi,
> +};
> +
> +#ifdef CONFIG_64BIT
> +#define clint_get_cycles()	readq_relaxed(clint_time_val)
> +#else
> +#define clint_get_cycles()	readl_relaxed(clint_time_val)
> +#define clint_get_cycles_hi()	readl_relaxed(((u32 *)clint_time_val) + 1)
> +#endif
> +
> +#ifdef CONFIG_64BIT
> +static u64 clint_get_cycles64(void)
> +{
> +	return clint_get_cycles();
> +}
> +#else /* CONFIG_64BIT */
> +static u64 clint_get_cycles64(void)
> +{
> +	u32 hi, lo;
> +
> +	do {
> +		hi = clint_get_cycles_hi();
> +		lo = clint_get_cycles();
> +	} while (hi != clint_get_cycles_hi());
> +
> +	return ((u64)hi << 32) | lo;
> +}
> +#endif /* CONFIG_64BIT */
> +
> +static int clint_clock_next_event(unsigned long delta,
> +				   struct clock_event_device *ce)
> +{
> +	void __iomem *r = clint_time_cmp +
> +			  cpuid_to_hartid_map(smp_processor_id());
> +
> +	csr_set(CSR_IE, IE_TIE);
> +	writeq_relaxed(clint_get_cycles64() + delta, r);
> +	return 0;
> +}
> +
> +static DEFINE_PER_CPU(struct clock_event_device, clint_clock_event) = {
> +	.name			= "clint_clockevent",
> +	.features		= CLOCK_EVT_FEAT_ONESHOT,
> +	.rating		= 100,
> +	.set_next_event	= clint_clock_next_event,
> +};
> +
> +static u64 clint_rdtime(struct clocksource *cs)
> +{
> +	return readq_relaxed(clint_time_val);
> +}
> +
> +static u64 notrace clint_sched_clock(void)
> +{
> +	return readq_relaxed(clint_time_val);
> +}
> +
> +static struct clocksource clint_clocksource = {
> +	.name		= "clint_clocksource",
> +	.rating	= 300,
> +	.mask		= CLOCKSOURCE_MASK(64),
> +	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
> +	.read		= clint_rdtime,
> +};
> +
> +static int clint_timer_starting_cpu(unsigned int cpu)
> +{
> +	struct clock_event_device *ce = per_cpu_ptr(&clint_clock_event, cpu);
> +
> +	ce->cpumask = cpumask_of(cpu);
> +	clockevents_config_and_register(ce, clint_freq, 200, ULONG_MAX);
> +
> +	enable_percpu_irq(clint_irq, irq_get_trigger_type(clint_irq));
> +	return 0;
> +}
> +
> +static int clint_timer_dying_cpu(unsigned int cpu)
> +{
> +	disable_percpu_irq(clint_irq);
> +	return 0;
> +}
> +
> +static irqreturn_t clint_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evdev = this_cpu_ptr(&clint_clock_event);
> +
> +	csr_clear(CSR_IE, IE_TIE);
> +	evdev->event_handler(evdev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __init clint_timer_init_dt(struct device_node *np)
> +{
> +	int rc;
> +	u32 i, nr_irqs;
> +	void __iomem *base;
> +	struct of_phandle_args oirq;
> +
> +	/*
> +	 * Ensure that CLINT device interrupts are either RV_IRQ_TIMER or
> +	 * RV_IRQ_SOFT. If it's anything else then we ignore the device.
> +	 */
> +	nr_irqs = of_irq_count(np);
> +	for (i = 0; i < nr_irqs; i++) {
> +		if (of_irq_parse_one(np, i, &oirq)) {
> +			pr_err("%pOFP: failed to parse irq %d.\n", np, i);
> +			continue;
> +		}
> +
> +		if ((oirq.args_count != 1) ||
> +		    (oirq.args[0] != RV_IRQ_TIMER &&
> +		     oirq.args[0] != RV_IRQ_SOFT)) {
> +			pr_info("%pOFP: invalid irq %d (hwirq %d)\n",
> +				np, i, oirq.args[0]);
> +			return 0;
> +		}
> +	}
> +
> +	oirq.np = riscv_of_intc_domain_node();
> +	oirq.args_count = 1;
> +	oirq.args[0] = RV_IRQ_TIMER;
> +	clint_irq = irq_create_of_mapping(&oirq);
> +	if (!clint_irq) {
> +		pr_err("%pOFP: could not map hwirq %d\n", np, RV_IRQ_TIMER);
> +		return -ENODEV;
> +	}
> +
> +	base = of_iomap(np, 0);
> +	if (!base) {
> +		pr_err("%pOFP: could not map registers\n", np);
> +		return -ENODEV;
> +	}
> +
> +	clint_ipi_base = base + CLINT_IPI_OFF;
> +	clint_time_cmp = base + CLINT_TIME_CMP_OFF;
> +	clint_time_val = base + CLINT_TIME_VAL_OFF;
> +	clint_freq = riscv_timebase;
> +
> +	pr_info("%pOFP: timer running at %ld Hz\n", np, clint_freq);
> +
> +	rc = clocksource_register_hz(&clint_clocksource, clint_freq);
> +	if (rc) {
> +		iounmap(base);
> +		pr_err("%pOFP: clocksource register failed [%d]\n", np, rc);
> +		return rc;
> +	}
> +
> +	sched_clock_register(clint_sched_clock, 64, clint_freq);
> +
> +	rc = request_percpu_irq(clint_irq, clint_timer_interrupt,
> +				 "clint-timer", &clint_clock_event);
> +	if (rc) {
> +		iounmap(base);
> +		pr_err("registering percpu irq failed [%d]\n", rc);
> +		return rc;
> +	}
> +
> +	rc = cpuhp_setup_state(CPUHP_AP_CLINT_TIMER_STARTING,
> +				"clockevents/clint/timer:starting",
> +				clint_timer_starting_cpu,
> +				clint_timer_dying_cpu);
> +	if (rc) {
> +		free_irq(clint_irq, &clint_clock_event);
> +		iounmap(base);
> +		pr_err("%pOFP: cpuhp setup state failed [%d]\n", np, rc);
> +		return rc;
> +	}
> +
> +	riscv_set_ipi_ops(&clint_ipi_ops);
> +	clint_clear_ipi();
> +
> +	return 0;
> +}
> +
> +TIMER_OF_DECLARE(clint_timer, "riscv,clint0", clint_timer_init_dt);
> +TIMER_OF_DECLARE(clint_timer1, "sifive,clint-1.0.0", clint_timer_init_dt);
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 57b1f8f777d9..52552492c2f2 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -132,6 +132,7 @@ enum cpuhp_state {
>  	CPUHP_AP_MIPS_GIC_TIMER_STARTING,
>  	CPUHP_AP_ARC_TIMER_STARTING,
>  	CPUHP_AP_RISCV_TIMER_STARTING,
> +	CPUHP_AP_CLINT_TIMER_STARTING,
>  	CPUHP_AP_CSKY_TIMER_STARTING,
>  	CPUHP_AP_HYPERV_TIMER_STARTING,
>  	CPUHP_AP_KVM_STARTING,

^ permalink raw reply

* Re: [PATCH 5/5] dt-bindings: timer: Add CLINT bindings
From: Palmer Dabbelt @ 2020-06-04 20:40 UTC (permalink / raw)
  To: Anup Patel
  Cc: Paul Walmsley, aou, robh+dt, daniel.lezcano, tglx, Damien Le Moal,
	Atish Patra, Alistair Francis, anup, linux-riscv, linux-kernel,
	devicetree, Anup Patel
In-Reply-To: <20200521134544.816918-6-anup.patel@wdc.com>

On Thu, 21 May 2020 06:45:44 PDT (-0700), Anup Patel wrote:
> We add DT bindings documentation for CLINT device.
>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>
> ---
>  .../bindings/timer/sifive,clint.txt           | 33 +++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/sifive,clint.txt
>
> diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.txt b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> new file mode 100644
> index 000000000000..cae2dad1223a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/sifive,clint.txt
> @@ -0,0 +1,33 @@
> +SiFive Core Local Interruptor (CLINT)
> +-------------------------------------
> +
> +SiFive (and other RISC-V) SOCs include an implementation of the SiFive Core
> +Local Interruptor (CLINT) for M-mode timer and inter-processor interrupts.
> +
> +It directly connects to the timer and inter-processor interrupt lines of
> +various HARTs (or CPUs) so RISC-V per-HART (or per-CPU) local interrupt
> +controller is the parent interrupt controller for CLINT device.
> +
> +The clock frequency of CLINT is specified via "timebase-frequency" DT
> +property of "/cpus" DT node. The "timebase-frequency" DT property is
> +described in: Documentation/devicetree/bindings/riscv/cpus.yaml
> +
> +Required properties:
> +- compatible : "sifive,clint-1.0.0" and a string identifying the actual
> +  detailed implementation in case that specific bugs need to be worked around.
> +- reg : Should contain 1 register range (address and length).
> +- interrupts-extended : Specifies which HARTs (or CPUs) are connected to
> +  the CLINT.  Each node pointed to should be a riscv,cpu-intc node, which
> +  has a riscv node as parent.
> +
> +Example:
> +
> +	clint@2000000 {
> +		compatible = "sifive,clint-1.0.0", "sifive,fu540-c000-clint";
> +		interrupts-extended = <
> +			&cpu1-intc 3 &cpu1-intc 7
> +			&cpu2-intc 3 &cpu2-intc 7
> +			&cpu3-intc 3 &cpu3-intc 7
> +			&cpu4-intc 3 &cpu4-intc 7>;
> +		reg = <0x2000000 0x4000000>;
> +	};

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>

^ permalink raw reply

* Re: [PATCH v3 12/13] dt-bindings: clock: Add Marvell MMP Audio Clock Controller binding
From: Rob Herring @ 2020-06-04 20:47 UTC (permalink / raw)
  To: Lubomir Rintel
  Cc: Stephen Boyd, Michael Turquette, linux-clk, devicetree,
	linux-kernel@vger.kernel.org,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20200519224151.2074597-13-lkundrak@v3.sk>

On Tue, May 19, 2020 at 4:42 PM Lubomir Rintel <lkundrak@v3.sk> wrote:
>
> This describes the bindings for a controller that generates master and bit
> clocks for the I2S interface.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
>
> ---
> Changes since v1:
> - Fix commit message wording
> - Define MMP2_CLK_AUDIO_NR_CLKS
> - Make clock ids start at 0, not 1
> - Fix dt-bindings/clock/marvell,mmp2-audio.h file name
> - Rename node from "clocks" to "clock-controller"
>
>  .../clock/marvell,mmp2-audio-clock.yaml       | 74 +++++++++++++++++++
>  .../dt-bindings/clock/marvell,mmp2-audio.h    | 10 +++
>  2 files changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
>  create mode 100644 include/dt-bindings/clock/marvell,mmp2-audio.h
>
> diff --git a/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> new file mode 100644
> index 000000000000..ab6e82d1d3a9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/marvell,mmp2-audio-clock.yaml
> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/marvell,mmp2-audio-clock.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell MMP2 Audio Clock Controller
> +
> +maintainers:
> +  - Lubomir Rintel <lkundrak@v3.sk>
> +
> +description: |
> +  The audio clock controller generates and supplies the clocks to the audio
> +  codec.
> +
> +  Each clock is assigned an identifier and client nodes use this identifier
> +  to specify the clock which they consume.
> +
> +  All these identifiers could be found in
> +  <dt-bindings/clock/marvell,mmp2-audio.h>.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - marvell,mmp2-audio-clock
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Audio subsystem clock
> +      - description: The crystal oscillator clock
> +      - description: First I2S clock
> +      - description: Second I2S clock
> +
> +  clock-names:
> +    items:
> +      - const: audio
> +      - const: vctcxo
> +      - const: i2s0
> +      - const: i2s1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/marvell,mmp2-audio.h>
> +    #include <dt-bindings/power/marvell,mmp2.h>
> +
> +    clock-controller@d42a0c30 {
> +      compatible = "marvell,mmp2-audio-clock";
> +      reg = <0xd42a0c30 0x10>;
> +      clock-names = "audio", "vctcxo", "i2s0", "i2s1";
> +      clocks = <&soc_clocks MMP2_CLK_AUDIO>,
> +               <&soc_clocks MMP2_CLK_VCTCXO>,
> +               <&soc_clocks MMP2_CLK_I2S0>,
> +               <&soc_clocks MMP2_CLK_I2S1>;

This now breaks linux-next. I think the above defines are missing
their include.

My testing wasn't happy either because it couldn't find
marvell,mmp2.h. I guess that's somewhere in linux-next and now we're
on to the secondary issue. Once that's fixed, then the schema checks
will actually run (hint: make sure they pass).

Please get this fixed or revert before it is sent to Linus. Maybe we
can have an rc1 without the schema broken.

Rob

^ permalink raw reply

* [PATCH v7 0/6] iommu/arm-smmu: Enable split pagetable support
From: Jordan Crouse @ 2020-06-04 20:57 UTC (permalink / raw)
  To: linux-arm-msm
  Cc: freedreno, Andy Gross, Bjorn Andersson, Brian Masney,
	Daniel Vetter, David Airlie, Douglas Anderson, Jeffrey Hugo,
	Joerg Roedel, Rob Clark, Rob Herring, Robin Murphy, Sean Paul,
	Takashi Iwai, Thomas Gleixner, Will Deacon, devicetree, dri-devel,
	iommu, linux-arm-kernel, linux-kernel

Another iteration of the split-pagetable support for arm-smmu and the Adreno GPU
SMMU. After email discussions [1] we opted to make a arm-smmu implementation for
specifically for the Adreno GPU and use that to enable split pagetable support
and later other implementation specific bits that we need.

On the hardware side this is very close to the same code from before [2] only
the TTBR1 quirk is turned on by the implementation and not a domain attribute.
In drm/msm we use the returned size of the aperture as a clue to let us know
which virtual address space we should use for global memory objects.

There are two open items that you should be aware of. First, in the
implementation specific code we have to check the compatible string of the
device so that we only enable TTBR1 for the GPU (SID 0) and not the GMU (SID 4).
I went back and forth trying to decide if I wanted to use the compatbile string
or the SID as the filter and settled on the compatible string but I could be
talked out of it.

The other open item is that in drm/msm the hardware only uses 49 bits of the
address space but arm-smmu expects the address to be sign extended all the way
to 64 bits. This isn't a problem normally unless you look at the hardware
registers that contain a IOVA and then the upper bits will be zero. I opted to
restrict the internal drm/msm IOVA range to only 49 bits and then sign extend
right before calling iommu_map / iommu_unmap. This is a bit wonky but I thought
that matching the hardware would be less confusing when debugging a hang.

[1] https://lists.linuxfoundation.org/pipermail/iommu/2020-May/044537.html
[2] https://patchwork.kernel.org/patch/11482591/


Jordan Crouse (6):
  iommu/arm-smmu: Pass io-pgtable config to implementation specific
    function
  iommu/arm-smmu: Add support for split pagetables
  dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
  iommu/arm-smmu: Add implementation for the adreno GPU SMMU
  drm/msm: Set the global virtual address range from the IOMMU domain
  arm6: dts: qcom: sm845: Set the compatible string for the GPU SMMU

 .../devicetree/bindings/iommu/arm,smmu.yaml   |  4 ++
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |  2 +-
 drivers/gpu/drm/msm/adreno/adreno_gpu.c       | 13 ++++++-
 drivers/gpu/drm/msm/msm_iommu.c               |  7 ++++
 drivers/iommu/arm-smmu-impl.c                 |  6 ++-
 drivers/iommu/arm-smmu-qcom.c                 | 38 ++++++++++++++++++-
 drivers/iommu/arm-smmu.c                      | 32 +++++++++++-----
 drivers/iommu/arm-smmu.h                      | 29 ++++++++++----
 8 files changed, 108 insertions(+), 23 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v7 6/6] arm6: dts: qcom: sm845: Set the compatible string for the GPU SMMU
From: Jordan Crouse @ 2020-06-04 20:57 UTC (permalink / raw)
  To: linux-arm-msm
  Cc: freedreno, Andy Gross, Bjorn Andersson, Rob Herring, devicetree,
	linux-kernel
In-Reply-To: <20200604205710.3167-1-jcrouse@codeaurora.org>

Set the qcom,adreno-smmu compatible string for the GPU SMMU to enable
split pagetables.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 8eb5a31346d2..8b15cd74e9ba 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -3556,7 +3556,7 @@
 		};
 
 		adreno_smmu: iommu@5040000 {
-			compatible = "qcom,sdm845-smmu-v2", "qcom,smmu-v2";
+			compatible = "qcom,adreno-smmu", "qcom,smmu-v2";
 			reg = <0 0x5040000 0 0x10000>;
 			#iommu-cells = <1>;
 			#global-interrupts = <2>;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v7 3/6] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
From: Jordan Crouse @ 2020-06-04 20:57 UTC (permalink / raw)
  To: linux-arm-msm
  Cc: freedreno, Joerg Roedel, Rob Herring, Robin Murphy, Will Deacon,
	devicetree, iommu, linux-arm-kernel, linux-kernel
In-Reply-To: <20200604205710.3167-1-jcrouse@codeaurora.org>

Every Qcom Adreno GPU has an embedded SMMU for its own use. These
devices depend on unique features such as split pagetables,
different stall/halt requirements and other settings. Identify them
with a compatible string so that they can be identified in the
arm-smmu implementation specific code.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index d7ceb4c34423..e52a1b146c97 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -38,6 +38,10 @@ properties:
               - qcom,sc7180-smmu-500
               - qcom,sdm845-smmu-500
           - const: arm,mmu-500
+      - description: Qcom Adreno GPUs implementing "arm,smmu-v2"
+        items:
+          - const: qcom,adreno-smmu
+          - const: qcom,smmu-v2
       - items:
           - const: arm,mmu-500
           - const: arm,smmu-v2
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH net-next v6 4/4] net: dp83869: Add RGMII internal delay configuration
From: kernel test robot @ 2020-06-04 21:09 UTC (permalink / raw)
  To: Dan Murphy, andrew, f.fainelli, hkallweit1, davem, robh
  Cc: kbuild-all, clang-built-linux, netdev, linux-kernel, devicetree,
	Dan Murphy
In-Reply-To: <20200604111410.17918-5-dmurphy@ti.com>

[-- Attachment #1: Type: text/plain, Size: 2023 bytes --]

Hi Dan,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on robh/for-next sparc-next/master net/master linus/master v5.7 next-20200604]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Dan-Murphy/RGMII-Internal-delay-common-property/20200605-003113
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2
config: x86_64-allmodconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ac47588bc4ff5927a01ed6fcd269ce86aba52a7c)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/net/phy/dp83869.c:103:18: warning: unused variable 'dp83869_internal_delay' [-Wunused-const-variable]
static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
^
1 warning generated.

vim +/dp83869_internal_delay +103 drivers/net/phy/dp83869.c

   102	
 > 103	static const int dp83869_internal_delay[] = {250, 500, 750, 1000, 1250, 1500,
   104						     1750, 2000, 2250, 2500, 2750, 3000,
   105						     3250, 3500, 3750, 4000};
   106	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74498 bytes --]

^ permalink raw reply

* [PATCH v4 00/11] Add support for Kontron sl28cpld
From: Michael Walle @ 2020-06-04 21:10 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Michael Walle

The Kontron sl28cpld is a board management chip providing gpio, pwm, fan
monitoring and an interrupt controller. For now this controller is used on
the Kontron SMARC-sAL28 board. But because of its flexible nature, it
might also be used on other boards in the future. The individual blocks
(like gpio, pwm, etc) are kept intentionally small. The MFD core driver
then instantiates different (or multiple of the same) blocks. It also
provides the register layout so it might be updated in the future without a
device tree change; and support other boards with a different layout or
functionalities.

See also [1] for more information.

This is my first take of a MFD driver. I don't know whether the subsystem
maintainers should only be CCed on the patches which affect the subsystem
or on all patches for this series. I've chosen the latter so you can get a
more complete picture.

[1] https://lore.kernel.org/linux-devicetree/0e3e8204ab992d75aa07fc36af7e4ab2@walle.cc/

Changes since v3:
 - use of_platform_populate() to populate internal devices using the
   internal register offsets as unit-addresses
 - because we don't use mfd_cells anymore, we cannot use IORESOURCE_REG,
   but instead parse the reg property in each individual driver
 - dropped the following patches because they were already merged:
     gpiolib: Introduce gpiochip_irqchip_add_domain()
     gpio: add a reusable generic gpio_chip using regmap
 - dropped the following patches because they are no longer needed:
     include/linux/ioport.h: add helper to define REG resource constructs
     mfd: mfd-core: Don't overwrite the dma_mask of the child device
     mfd: mfd-core: match device tree node against reg property
 - rephrase commit messages, as suggested by Thomas Gleixner

Changes since v2:
As suggested by Guenter Roeck:
 - added sl28cpld.rst to index.rst
 - removed sl28cpld_wdt_status()
 - reverse christmas tree local variable ordering
 - assign device_property_read_bool() retval directly
 - introduce WDT_DEFAULT_TIMEOUT and use it if the hardware reports
   0 as timeout.
 - set WDOG_HW_RUNNING if applicable
 - remove platform_set_drvdata() leftover

As suggested by Bartosz Golaszewski:
 - don't export gpio_regmap_simple_xlate()
 - combine local variable declaration of the same type
 - drop the "struct gpio_regmap_addr", instead use -1 to force an address
   offset of zero
 - fix typo
 - use "struct gpio_regmap_config" pattern, keep "struct gpio_regmap"
   private. this also means we need a getter/setter for the driver_data
   element.

As suggested by Linus Walleij:
 - don't store irq_domain in gpio-regmap. drop to_irq() altogether for now.
   Instead there is now a new patch which lets us set the irqdomain of the
   gpiochip_irqchip and use its .to_irq() function. This way we don't have
   to expose the gpio_chip inside the gpio-regmap to the user.

Changes since v1:
 - use of_match_table in all drivers, needed for automatic module loading,
   when using OF_MFD_CELL()
 - add new gpio-regmap.c which adds a generic regmap gpio_chip
   implementation
 - new patch for reqmap_irq, so we can reuse its implementation
 - remove almost any code from gpio-sl28cpld.c, instead use gpio-regmap and
   regmap-irq
 - change the handling of the mfd core vs device tree nodes; add a new
   property "of_reg" to the mfd_cell struct which, when set, is matched to
   the unit-address of the device tree nodes.
 - fix sl28cpld watchdog when it is not initialized by the bootloader.
   Explicitly set the operation mode.
 - also add support for kontron,assert-wdt-timeout-pin in sl28cpld-wdt.

As suggested by Bartosz Golaszewski:
 - define registers as hex
 - make gpio enum uppercase
 - move parent regmap check before memory allocation
 - use device_property_read_bool() instead of the of_ version
 - mention the gpio flavors in the bindings documentation

As suggested by Guenter Roeck:
 - cleanup #includes and sort them
 - use devm_watchdog_register_device()
 - use watchdog_stop_on_reboot()
 - provide a Documentation/hwmon/sl28cpld.rst
 - cleaned up the weird tristate->bool and I2C=y issue. Instead mention
   that the MFD driver is bool because of the following intc patch
 - removed the SL28CPLD_IRQ typo

As suggested by Rob Herring:
 - combine all dt bindings docs into one patch
 - change the node name for all gpio flavors to "gpio"
 - removed the interrupts-extended rule
 - cleaned up the unit-address space, see above

Michael Walle (11):
  dt-bindings: mfd: Add bindings for sl28cpld
  mfd: Add support for Kontron sl28cpld management controller
  irqchip: add sl28cpld interrupt controller support
  watchdog: add support for sl28cpld watchdog
  pwm: add support for sl28cpld PWM controller
  gpio: add support for the sl28cpld GPIO controller
  hwmon: add support for the sl28cpld hardware monitoring controller
  arm64: dts: freescale: sl28: enable sl28cpld
  arm64: dts: freescale: sl28: map GPIOs to input events
  arm64: dts: freescale: sl28: enable LED support
  arm64: dts: freescale: sl28: enable fan support

 .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 ++++
 .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 ++
 .../kontron,sl28cpld-intc.yaml                |  54 ++++
 .../bindings/mfd/kontron,sl28cpld.yaml        | 153 ++++++++++++
 .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 +++
 .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 +++
 Documentation/hwmon/index.rst                 |   1 +
 Documentation/hwmon/sl28cpld.rst              |  36 +++
 .../fsl-ls1028a-kontron-kbox-a-230-ls.dts     |  14 ++
 .../fsl-ls1028a-kontron-sl28-var3-ads2.dts    |   9 +
 .../freescale/fsl-ls1028a-kontron-sl28.dts    | 134 ++++++++++
 drivers/gpio/Kconfig                          |  11 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-sl28cpld.c                  | 180 ++++++++++++++
 drivers/hwmon/Kconfig                         |  10 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/sl28cpld-hwmon.c                | 150 ++++++++++++
 drivers/irqchip/Kconfig                       |   3 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-sl28cpld.c                | 102 ++++++++
 drivers/mfd/Kconfig                           |  21 ++
 drivers/mfd/Makefile                          |   2 +
 drivers/mfd/sl28cpld.c                        |  79 ++++++
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-sl28cpld.c                    | 201 +++++++++++++++
 drivers/watchdog/Kconfig                      |  11 +
 drivers/watchdog/Makefile                     |   1 +
 drivers/watchdog/sl28cpld_wdt.c               | 231 ++++++++++++++++++
 29 files changed, 1568 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
 create mode 100644 Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
 create mode 100644 Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
 create mode 100644 Documentation/hwmon/sl28cpld.rst
 create mode 100644 drivers/gpio/gpio-sl28cpld.c
 create mode 100644 drivers/hwmon/sl28cpld-hwmon.c
 create mode 100644 drivers/irqchip/irq-sl28cpld.c
 create mode 100644 drivers/mfd/sl28cpld.c
 create mode 100644 drivers/pwm/pwm-sl28cpld.c
 create mode 100644 drivers/watchdog/sl28cpld_wdt.c

-- 
2.20.1


^ permalink raw reply

* [PATCH v4 10/11] arm64: dts: freescale: sl28: enable LED support
From: Michael Walle @ 2020-06-04 21:10 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Michael Walle
In-Reply-To: <20200604211039.12689-1-michael@walle.cc>

Now that we have support for GPIO lines of the SMARC connector, enable
LED support on the KBox A-230-LS. There are two LEDs without fixed
functions, one is yellow and one is green. Unfortunately, it is just one
multi-color LED, thus while it is possible to enable both at the same
time it is hard to tell the difference between "yellow only" and "yellow
and green".

Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../fsl-ls1028a-kontron-kbox-a-230-ls.dts          | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
index 4b4cc6a1573d..49cf4fe05c80 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-kbox-a-230-ls.dts
@@ -16,6 +16,20 @@
 	model = "Kontron KBox A-230-LS";
 	compatible = "kontron,kbox-a-230-ls", "kontron,sl28-var4",
 		     "kontron,sl28", "fsl,ls1028a";
+
+	leds {
+		compatible = "gpio-leds";
+
+		user_yellow {
+			label = "s1914:yellow:user";
+			gpios = <&sl28cpld_gpio0 0 0>;
+		};
+
+		user_green {
+			label = "s1914:green:user";
+			gpios = <&sl28cpld_gpio1 3 0>;
+		};
+	};
 };
 
 &enetc_mdio_pf3 {
-- 
2.20.1


^ permalink raw reply related

* [PATCH v4 08/11] arm64: dts: freescale: sl28: enable sl28cpld
From: Michael Walle @ 2020-06-04 21:10 UTC (permalink / raw)
  To: linux-gpio, devicetree, linux-kernel, linux-hwmon, linux-pwm,
	linux-watchdog, linux-arm-kernel
  Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring, Jean Delvare,
	Guenter Roeck, Lee Jones, Thierry Reding, Uwe Kleine-König,
	Wim Van Sebroeck, Shawn Guo, Li Yang, Thomas Gleixner,
	Jason Cooper, Marc Zyngier, Mark Brown, Greg Kroah-Hartman,
	Andy Shevchenko, Michael Walle
In-Reply-To: <20200604211039.12689-1-michael@walle.cc>

Add the board management controller node.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 .../freescale/fsl-ls1028a-kontron-sl28.dts    | 102 ++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
index 360b3a168c10..8712fe82727b 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28.dts
@@ -8,6 +8,7 @@
 
 /dts-v1/;
 #include "fsl-ls1028a.dtsi"
+#include <dt-bindings/interrupt-controller/irq.h>
 
 / {
 	model = "Kontron SMARC-sAL28";
@@ -170,6 +171,107 @@
 		reg = <0x32>;
 	};
 
+	sl28cpld@4a {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "kontron,sl28cpld-r1";
+		reg = <0x4a>;
+
+		watchdog@4 {
+			compatible = "kontron,sl28cpld-wdt";
+			reg = <0x4>;
+			kontron,assert-wdt-timeout-pin;
+		};
+
+		hwmon@b {
+			compatible = "kontron,sl28cpld-fan";
+			reg = <0xb>;
+		};
+
+		sl28cpld_pwm0: pwm@c {
+			#pwm-cells = <2>;
+			compatible = "kontron,sl28cpld-pwm";
+			reg = <0xc>;
+		};
+
+		sl28cpld_pwm1: pwm@e {
+			#pwm-cells = <2>;
+			compatible = "kontron,sl28cpld-pwm";
+			reg = <0xe>;
+		};
+
+		sl28cpld_gpio0: gpio@10 {
+			compatible = "kontron,sl28cpld-gpio";
+			reg = <0x10>;
+			interrupts-extended = <&gpio2 6
+					       IRQ_TYPE_EDGE_FALLING>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"GPIO0_CAM0_PWR_N", "GPIO1_CAM1_PWR_N",
+				"GPIO2_CAM0_RST_N", "GPIO3_CAM1_RST_N",
+				"GPIO4_HDA_RST_N", "GPIO5_PWM_OUT",
+				"GPIO6_TACHIN", "GPIO7";
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		sl28cpld_gpio1: gpio@15 {
+			compatible = "kontron,sl28cpld-gpio";
+			reg = <0x15>;
+			interrupts-extended = <&gpio2 6
+					       IRQ_TYPE_EDGE_FALLING>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"GPIO8", "GPIO9", "GPIO10", "GPIO11",
+				"", "", "", "";
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+
+		sl28cpld_gpio2: gpio@1a {
+			compatible = "kontron,sl28cpld-gpo";
+			reg = <0x1a>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"LCD0 voltage enable",
+				"LCD0 backlight enable",
+				"eMMC reset", "LVDS bridge reset",
+				"LVDS bridge power-down",
+				"SDIO power enable",
+				"", "";
+		};
+
+		sl28cpld_gpio3: gpio@1b {
+			compatible = "kontron,sl28cpld-gpi";
+			reg = <0x1b>;
+
+			gpio-controller;
+			#gpio-cells = <2>;
+			gpio-line-names =
+				"Power button", "Force recovery", "Sleep",
+				"Battery low", "Lid state", "Charging",
+				"Charger present", "";
+		};
+
+		sl28cpld_intc: interrupt-controller@1c {
+			compatible = "kontron,sl28cpld-intc";
+			reg = <0x1c>;
+			interrupts-extended = <&gpio2 6
+					       IRQ_TYPE_EDGE_FALLING>;
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+		};
+	};
+
 	eeprom@50 {
 		compatible = "atmel,24c32";
 		reg = <0x50>;
-- 
2.20.1


^ permalink raw reply related


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