Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH -next] input: keyboard: remove duplicated include from mtk-pmic-keys.c
From: Lee Jones @ 2018-12-10 10:07 UTC (permalink / raw)
  To: YueHaibing
  Cc: dmitry.torokhov, linux-kernel, linux-mediatek, linux-input,
	matthias.bgg, chen.zhong, linux-arm-kernel
In-Reply-To: <4f1a67e5-b735-f8bd-88b8-3a1ca590eba2@huawei.com>

On Mon, 10 Dec 2018, YueHaibing wrote:

> On 2018/12/10 14:15, Lee Jones wrote:
> > On Sun, 09 Dec 2018, YueHaibing wrote:
> > 
> >> Remove duplicated include.
> >>
> >> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> >> ---
> >>  drivers/input/keyboard/mtk-pmic-keys.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> >> index 02c67a1..5027ebb 100644
> >> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> >> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
> >> @@ -19,7 +19,6 @@
> >>  #include <linux/input.h>
> >>  #include <linux/interrupt.h>
> >>  #include <linux/platform_device.h>
> >> -#include <linux/kernel.h>
> >>  #include <linux/of.h>
> >>  #include <linux/of_device.h>
> >>  #include <linux/regmap.h>
> > 
> > You are removing the wrong one.
> 
> No, linux/kernel.h is a duplicated include indeed.

Actually both are not correct, but the first instance (at the top) is
even more incorrect.

> > Please convert this patch's main intent to alphabetise the header
> > files.  Then you can remove any obvious duplicates.
> 
> I can alphabetize it in v2 if need be.

Yes please.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3] drm/rockchip: support hwc layer
From: Heiko Stuebner @ 2018-12-10 10:06 UTC (permalink / raw)
  To: Randy Li
  Cc: airlied, linux-kernel, dri-devel, hjc, linux-rockchip,
	linux-arm-kernel
In-Reply-To: <20181205135230.17677-1-ayaka@soulik.info>

Am Mittwoch, 5. Dezember 2018, 14:52:30 CET schrieb Randy Li:
> From: ayaka <ayaka@soulik.info>
> 
> The Windows 2/3 or a RGB UI layer is a high performance flexibly
> plane. It is too waste to use it as a cursor plane.
> 
> I have verified this patch with weston git version, I am not
> sure whether X would meet with this patch. As the previous
> author is gone, I can't confirm this problem with him.
> 
> Also the weston only use the only two achors with a same
> size and pixel format, I need more users to verify this
> patch.
> 
> changelog:
> v2: the previous version is mixed with the code for the other
> patches, I forget to remove it.
> v3: fix the error for the rk3399 vop little.
> 
> Signed-off-by: Randy Li <ayaka@soulik.info>

just as a note for people not following irc discussions [and also for
my bad memory :-) ], it got reported that while this seems to work
nice on wayland, X11 seems to have an issue with it, to quote from
the public log:

"hmm.. but something funky is going on. with your patch the curser doesn't
seem to be updated if i move it close to the left or right edges of the
screen if the slower i move the curser towards an edge the closer to the
edge it will draw the cursor removing your patch again fixes the problem
this is in plain old Xorg version 1.20.3"


>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 20 ++++++++++++++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 42 ++++++++++++++++++++++++++---
>  2 files changed, 58 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index fb70fb486fbf..1a3b72391ee8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -751,6 +751,26 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
>  
>  	dsp_info = (drm_rect_height(dest) - 1) << 16;
>  	dsp_info |= (drm_rect_width(dest) - 1) & 0xffff;
> +	/* HWC layer only supports various of square icon */
> +	if (plane->type == DRM_PLANE_TYPE_CURSOR) {
> +		switch (actual_w) {
> +		case 32:
> +			dsp_info = 0;
> +			break;
> +		case 64:
> +			dsp_info = 0x1;
> +			break;
> +		case 94:
> +			dsp_info = 0x10;
> +			break;
> +		case 128:
> +			dsp_info = 0x11;
> +			break;
> +		/* Unsupported pixel resolution */
> +		default:
> +			return;
> +		}
> +	}
>  
>  	dsp_stx = dest->x1 + crtc->mode.htotal - crtc->mode.hsync_start;
>  	dsp_sty = dest->y1 + crtc->mode.vtotal - crtc->mode.vsync_start;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 08fc40af52c8..694f43fdeb23 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -63,6 +63,15 @@ static const uint32_t formats_win_lite[] = {
>  	DRM_FORMAT_BGR565,
>  };
>  
> +static const uint32_t formats_win_hwc[] = {
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_RGB888,
> +	DRM_FORMAT_BGR888,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_BGR565,
> +};
> +
>  static const struct vop_scl_regs rk3036_win_scl = {
>  	.scale_yrgb_x = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 0x0),
>  	.scale_yrgb_y = VOP_REG(RK3036_WIN0_SCL_FACTOR_YRGB, 0xffff, 16),
> @@ -456,6 +465,19 @@ static const struct vop_win_phy rk3288_win23_data = {
>  	.dst_alpha_ctl = VOP_REG(RK3288_WIN2_DST_ALPHA_CTRL, 0xff, 0),
>  };
>  
> +static const struct vop_win_phy rk3288_winhwc_data = {
> +	.data_formats = formats_win_hwc,
> +	.nformats = ARRAY_SIZE(formats_win_hwc),
> +	.enable = VOP_REG(RK3288_HWC_CTRL0, 0x1, 0),
> +	.format = VOP_REG(RK3288_HWC_CTRL0, 0x7, 1),
> +	.rb_swap = VOP_REG(RK3288_HWC_CTRL0, 0x1, 12),
> +	.dsp_info = VOP_REG(RK3288_HWC_CTRL0, 0x3, 5),
> +	.dsp_st = VOP_REG(RK3288_HWC_DSP_ST, 0x1fff1fff, 0),
> +	.yrgb_mst = VOP_REG(RK3288_HWC_MST, 0xffffffff, 0),
> +	.src_alpha_ctl = VOP_REG(RK3288_HWC_SRC_ALPHA_CTRL, 0xffff, 0),
> +	.dst_alpha_ctl = VOP_REG(RK3288_HWC_DST_ALPHA_CTRL, 0xffffffff, 0),
> +};
> +
>  static const struct vop_modeset rk3288_modeset = {
>  	.htotal_pw = VOP_REG(RK3288_DSP_HTOTAL_HS_END, 0x1fff1fff, 0),
>  	.hact_st_end = VOP_REG(RK3288_DSP_HACT_ST_END, 0x1fff1fff, 0),
> @@ -500,7 +522,10 @@ static const struct vop_win_data rk3288_vop_win_data[] = {
>  	{ .base = 0x00, .phy = &rk3288_win23_data,
>  	  .type = DRM_PLANE_TYPE_OVERLAY },
>  	{ .base = 0x50, .phy = &rk3288_win23_data,
> -	  .type = DRM_PLANE_TYPE_CURSOR },
> +	  .type = DRM_PLANE_TYPE_OVERLAY },
> +	{ .base = 0x00, .phy = &rk3288_winhwc_data,
> +	  .type = DRM_PLANE_TYPE_CURSOR,
> +	},
>  };
>  
>  static const int rk3288_vop_intrs[] = {
> @@ -573,7 +598,10 @@ static const struct vop_win_data rk3368_vop_win_data[] = {
>  	{ .base = 0x00, .phy = &rk3368_win23_data,
>  	  .type = DRM_PLANE_TYPE_OVERLAY },
>  	{ .base = 0x50, .phy = &rk3368_win23_data,
> -	  .type = DRM_PLANE_TYPE_CURSOR },
> +	  .type = DRM_PLANE_TYPE_OVERLAY },
> +	{ .base = 0x00, .phy = &rk3288_winhwc_data,
> +	  .type = DRM_PLANE_TYPE_CURSOR,
> +	},
>  };
>  
>  static const struct vop_output rk3368_output = {
> @@ -653,7 +681,10 @@ static const struct vop_win_data rk3399_vop_lit_win_data[] = {
>  	{ .base = 0x00, .phy = &rk3288_win01_data,
>  	  .type = DRM_PLANE_TYPE_PRIMARY },
>  	{ .base = 0x00, .phy = &rk3368_win23_data,
> -	  .type = DRM_PLANE_TYPE_CURSOR},
> +	  .type = DRM_PLANE_TYPE_OVERLAY},
> +	{ .base = 0x00, .phy = &rk3288_winhwc_data,
> +	  .type = DRM_PLANE_TYPE_CURSOR,
> +	},
>  };
>  
>  static const struct vop_data rk3399_vop_lit = {
> @@ -735,7 +766,10 @@ static const struct vop_win_data rk3328_vop_win_data[] = {
>  	{ .base = 0x1d0, .phy = &rk3288_win01_data,
>  	  .type = DRM_PLANE_TYPE_OVERLAY },
>  	{ .base = 0x2d0, .phy = &rk3288_win01_data,
> -	  .type = DRM_PLANE_TYPE_CURSOR },
> +	  .type = DRM_PLANE_TYPE_OVERLAY },
> +	{ .base = 0x3d0, .phy = &rk3288_winhwc_data,
> +	  .type = DRM_PLANE_TYPE_CURSOR,
> +	},
>  };
>  
>  static const struct vop_data rk3328_vop = {
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 1/8] arm64: KVM: Make VHE Stage-2 TLB invalidation operations non-interruptible
From: Christoffer Dall @ 2018-12-10 10:03 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Mark Rutland, kvm, Suzuki K Poulose, Catalin Marinas, Will Deacon,
	James Morse, kvmarm, linux-arm-kernel
In-Reply-To: <20181206173126.139877-2-marc.zyngier@arm.com>

On Thu, Dec 06, 2018 at 05:31:19PM +0000, Marc Zyngier wrote:
> Contrary to the non-VHE version of the TLB invalidation helpers, the VHE
> code  has interrupts enabled, meaning that we can take an interrupt in
> the middle of such a sequence, and start running something else with
> HCR_EL2.TGE cleared.

Do we have to clear TGE to perform the TLB invalidation, or is that just
a side-effect of re-using code?

Also, do we generally perform TLB invalidations in the kernel with
interrupts disabled, or is this just a result of clearing TGE?

Somehow I feel like this should look more like just another TLB
invalidation in the kernel, but if there's a good reason why it can't
then this is fine.

Thanks,

    Christoffer

> 
> That's really not a good idea.
> 
> Take the heavy-handed option and disable interrupts in
> __tlb_switch_to_guest_vhe, restoring them in __tlb_switch_to_host_vhe.
> The latter also gain an ISB in order to make sure that TGE really has
> taken effect.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  arch/arm64/kvm/hyp/tlb.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/kvm/hyp/tlb.c b/arch/arm64/kvm/hyp/tlb.c
> index 4dbd9c69a96d..7fcc9c1a5f45 100644
> --- a/arch/arm64/kvm/hyp/tlb.c
> +++ b/arch/arm64/kvm/hyp/tlb.c
> @@ -15,14 +15,19 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <linux/irqflags.h>
> +
>  #include <asm/kvm_hyp.h>
>  #include <asm/kvm_mmu.h>
>  #include <asm/tlbflush.h>
>  
> -static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm,
> +						 unsigned long *flags)
>  {
>  	u64 val;
>  
> +	local_irq_save(*flags);
> +
>  	/*
>  	 * With VHE enabled, we have HCR_EL2.{E2H,TGE} = {1,1}, and
>  	 * most TLB operations target EL2/EL0. In order to affect the
> @@ -37,7 +42,8 @@ static void __hyp_text __tlb_switch_to_guest_vhe(struct kvm *kvm)
>  	isb();
>  }
>  
> -static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_guest_nvhe(struct kvm *kvm,
> +						  unsigned long *flags)
>  {
>  	__load_guest_stage2(kvm);
>  	isb();
> @@ -48,7 +54,8 @@ static hyp_alternate_select(__tlb_switch_to_guest,
>  			    __tlb_switch_to_guest_vhe,
>  			    ARM64_HAS_VIRT_HOST_EXTN);
>  
> -static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm,
> +						unsigned long flags)
>  {
>  	/*
>  	 * We're done with the TLB operation, let's restore the host's
> @@ -56,9 +63,12 @@ static void __hyp_text __tlb_switch_to_host_vhe(struct kvm *kvm)
>  	 */
>  	write_sysreg(0, vttbr_el2);
>  	write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2);
> +	isb();
> +	local_irq_restore(flags);
>  }
>  
> -static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm)
> +static void __hyp_text __tlb_switch_to_host_nvhe(struct kvm *kvm,
> +						 unsigned long flags)
>  {
>  	write_sysreg(0, vttbr_el2);
>  }
> @@ -70,11 +80,13 @@ static hyp_alternate_select(__tlb_switch_to_host,
>  
>  void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  {
> +	unsigned long flags;
> +
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	/*
>  	 * We could do so much better if we had the VA as well.
> @@ -117,36 +129,39 @@ void __hyp_text __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa)
>  	if (!has_vhe() && icache_is_vpipt())
>  		__flush_icache_all();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_vmid(struct kvm *kvm)
>  {
> +	unsigned long flags;
> +
>  	dsb(ishst);
>  
>  	/* Switch to requested VMID */
>  	kvm = kern_hyp_va(kvm);
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	__tlbi(vmalls12e1is);
>  	dsb(ish);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_tlb_flush_local_vmid(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm *kvm = kern_hyp_va(kern_hyp_va(vcpu)->kvm);
> +	unsigned long flags;
>  
>  	/* Switch to requested VMID */
> -	__tlb_switch_to_guest()(kvm);
> +	__tlb_switch_to_guest()(kvm, &flags);
>  
>  	__tlbi(vmalle1);
>  	dsb(nsh);
>  	isb();
>  
> -	__tlb_switch_to_host()(kvm);
> +	__tlb_switch_to_host()(kvm, flags);
>  }
>  
>  void __hyp_text __kvm_flush_vm_context(void)
> -- 
> 2.19.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 05/10] mailbox: tegra-hsp: Add suspend/resume support
From: Thierry Reding @ 2018-12-10  9:58 UTC (permalink / raw)
  To: Jon Hunter
  Cc: devicetree, Greg Kroah-Hartman, Jassi Brar, Mika Liljeberg,
	Mikko Perttunen, Timo Alho, linux-serial, Jiri Slaby, linux-tegra,
	Pekka Pessi, linux-arm-kernel
In-Reply-To: <9a7a55fa-9989-21cd-f981-f43d3b0f2d6c@nvidia.com>


[-- Attachment #1.1: Type: text/plain, Size: 2233 bytes --]

On Tue, Nov 13, 2018 at 11:17:58AM +0000, Jon Hunter wrote:
> 
> On 12/11/2018 15:18, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Upon resuming from a system sleep state, the interrupts for all active
> > shared mailboxes need to be reenabled, otherwise they will not work.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/mailbox/tegra-hsp.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> > index 0100a974149b..1259abf3542f 100644
> > --- a/drivers/mailbox/tegra-hsp.c
> > +++ b/drivers/mailbox/tegra-hsp.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/pm.h>
> >  #include <linux/slab.h>
> >  
> >  #include <dt-bindings/mailbox/tegra186-hsp.h>
> > @@ -817,6 +818,23 @@ static int tegra_hsp_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > +static int tegra_hsp_resume(struct device *dev)
> > +{
> > +	struct tegra_hsp *hsp = dev_get_drvdata(dev);
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < hsp->num_sm; i++) {
> > +		struct tegra_hsp_mailbox *mb = &hsp->mailboxes[i];
> > +
> > +		if (mb->channel.chan->cl)
> > +			tegra_hsp_mailbox_startup(mb->channel.chan);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(tegra_hsp_pm_ops, NULL, tegra_hsp_resume);
> 
> Is it worth disabling interrupts on suspend to avoid any in-flight
> interrupt triggering a bad access on entering suspend? I assume that the
> context of the mailbox registers get cleared/lost at some point and so I
> was not sure if there is a time where they have a bad state or are
> inaccessible?

Theoretically I think we'd have to do that. However, since we end up
having to busy-loop for any current use-cases (i.e. console) anyway,
we'll never end up in a situation where an interrupt could occur at
this point. The console will long have been suspended when we reach
this point. I'll take a look if a use-case can be constructed where
such an in-flight interrupt could be produced.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 08/15] irqchip: Add RDA8810PL interrupt driver
From: Manivannan Sadhasivam @ 2018-12-10  9:56 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: devicetree, jason, arnd, gregkh, linus.walleij, daniel.lezcano,
	linux-kernel, amit.kucheria, robh+dt, linux-serial, jslaby, olof,
	tglx, overseas.sales, afaerber, linux-arm-kernel, zhao_steven
In-Reply-To: <9debdd34-0c4a-a0a3-a284-b6372c2f7fd5@arm.com>

Hi Marc,

On Mon, Dec 10, 2018 at 09:39:13AM +0000, Marc Zyngier wrote:
> On 28/11/2018 13:50, Manivannan Sadhasivam wrote:
> > Add interrupt driver for RDA Micro RDA8810PL SoC.
> > 
> > Signed-off-by: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> Given the dependency on the platform Kconfig entry, how do you want this
> to be merged?
> 

Hmmm, I think I can just move the platform Kconfig change to SoC changes
in next revision so that you can apply the driver independently.

Regards,
Mani

> Thanks,
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2.1 24/34] dt-bindings: arm: Convert Rockchip board/soc bindings to json-schema
From: Heiko Stuebner @ 2018-12-10  9:54 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, devicetree, Kumar Gala, ARM-SoC Maintainers,
	Sean Hudson, linuxppc-dev, linux-kernel@vger.kernel.org,
	open list:ARM/Rockchip SoC..., Grant Likely, Matthias Brugger,
	Frank Rowand,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <2386212.ILKoqq0Ivo@phil>

Am Sonntag, 9. Dezember 2018, 23:14:05 CET schrieb Heiko Stuebner:
Forgot the

From: Rob Herring <robh@kernel.org>

here, but if you're ok with how it looks I can apply it to my tree.


Heiko

> Convert Rockchip SoC bindings to DT schema format using json-schema.
> 
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-rockchip@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> [move to per-board entries and added recently added boards]
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Hi Rob,
> 
> there are boards where the description adds much value and on others
> it is maybe less, but personally I'd like to keep things uniform,
> as that makes reading these things easier if the format stays the
> same all the time, so I've gone forward and just did the conversion
> 
> make dtbs_check did not complain about the schema it seems but I
> did end up with an error later on:
> 
> FATAL ERROR: Unknown output format "yaml"
> make[2]: *** [scripts/Makefile.lib:313: arch/arm/boot/dts/rk3036-evb.dt.yaml] Fehler 1
> 
> But I guess I did not mess up the schema yet.
> 
> So does it look ok that way?
> Heiko
> 
>  .../devicetree/bindings/arm/rockchip.txt      | 240 ----------
>  .../devicetree/bindings/arm/rockchip.yaml     | 419 ++++++++++++++++++
>  2 files changed, 419 insertions(+), 240 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/rockchip.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/rockchip.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt b/Documentation/devicetree/bindings/arm/rockchip.txt
> deleted file mode 100644
> index 0cc71236d639..000000000000
> --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> +++ /dev/null
> @@ -1,240 +0,0 @@
> -Rockchip platforms device tree bindings
> ----------------------------------------
> -
> -- 96boards RK3399 Ficus (ROCK960 Enterprise Edition)
> -    Required root node properties:
> -      - compatible = "vamrs,ficus", "rockchip,rk3399";
> -
> -- 96boards RK3399 Rock960 (ROCK960 Consumer Edition)
> -    Required root node properties:
> -      - compatible = "vamrs,rock960", "rockchip,rk3399";
> -
> -- Amarula Vyasa RK3288 board
> -    Required root node properties:
> -      - compatible = "amarula,vyasa-rk3288", "rockchip,rk3288";
> -
> -- Asus Tinker board
> -    Required root node properties:
> -      - compatible = "asus,rk3288-tinker", "rockchip,rk3288";
> -
> -- Asus Tinker board S
> -    Required root node properties:
> -      - compatible = "asus,rk3288-tinker-s", "rockchip,rk3288";
> -
> -- Kylin RK3036 board:
> -    Required root node properties:
> -      - compatible = "rockchip,kylin-rk3036", "rockchip,rk3036";
> -
> -- MarsBoard RK3066 board:
> -    Required root node properties:
> -      - compatible = "haoyu,marsboard-rk3066", "rockchip,rk3066a";
> -
> -- bq Curie 2 tablet:
> -    Required root node properties:
> -      - compatible = "mundoreader,bq-curie2", "rockchip,rk3066a";
> -
> -- ChipSPARK Rayeager PX2 board:
> -    Required root node properties:
> -      - compatible = "chipspark,rayeager-px2", "rockchip,rk3066a";
> -
> -- Radxa Rock board:
> -    Required root node properties:
> -      - compatible = "radxa,rock", "rockchip,rk3188";
> -
> -- Radxa Rock2 Square board:
> -    Required root node properties:
> -      - compatible = "radxa,rock2-square", "rockchip,rk3288";
> -
> -- Rikomagic MK808 v1 board:
> -    Required root node properties:
> -      - compatible = "rikomagic,mk808", "rockchip,rk3066a";
> -
> -- Firefly Firefly-RK3288 board:
> -    Required root node properties:
> -      - compatible = "firefly,firefly-rk3288", "rockchip,rk3288";
> -    or
> -      - compatible = "firefly,firefly-rk3288-beta", "rockchip,rk3288";
> -
> -- Firefly Firefly-RK3288 Reload board:
> -    Required root node properties:
> -      - compatible = "firefly,firefly-rk3288-reload", "rockchip,rk3288";
> -
> -- Firefly Firefly-RK3399 board:
> -    Required root node properties:
> -      - compatible = "firefly,firefly-rk3399", "rockchip,rk3399";
> -
> -- Firefly roc-rk3328-cc board:
> -    Required root node properties:
> -      - compatible = "firefly,roc-rk3328-cc", "rockchip,rk3328";
> -
> -- Firefly ROC-RK3399-PC board:
> -    Required root node properties:
> -      - compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
> -
> -- ChipSPARK PopMetal-RK3288 board:
> -    Required root node properties:
> -      - compatible = "chipspark,popmetal-rk3288", "rockchip,rk3288";
> -
> -- Netxeon R89 board:
> -    Required root node properties:
> -      - compatible = "netxeon,r89", "rockchip,rk3288";
> -
> -- GeekBuying GeekBox:
> -    Required root node properties:
> -      - compatible = "geekbuying,geekbox", "rockchip,rk3368";
> -
> -- Google Bob (Asus Chromebook Flip C101PA):
> -    Required root node properties:
> -	compatible = "google,bob-rev13", "google,bob-rev12",
> -		     "google,bob-rev11", "google,bob-rev10",
> -		     "google,bob-rev9", "google,bob-rev8",
> -		     "google,bob-rev7", "google,bob-rev6",
> -		     "google,bob-rev5", "google,bob-rev4",
> -		     "google,bob", "google,gru", "rockchip,rk3399";
> -
> -- Google Brain (dev-board):
> -    Required root node properties:
> -      - compatible = "google,veyron-brain-rev0", "google,veyron-brain",
> -		     "google,veyron", "rockchip,rk3288";
> -
> -- Google Gru (dev-board):
> -    Required root node properties:
> -      - compatible = "google,gru-rev15", "google,gru-rev14",
> -		     "google,gru-rev13", "google,gru-rev12",
> -		     "google,gru-rev11", "google,gru-rev10",
> -		     "google,gru-rev9", "google,gru-rev8",
> -		     "google,gru-rev7", "google,gru-rev6",
> -		     "google,gru-rev5", "google,gru-rev4",
> -		     "google,gru-rev3", "google,gru-rev2",
> -		     "google,gru", "rockchip,rk3399";
> -
> -- Google Jaq (Haier Chromebook 11 and more):
> -    Required root node properties:
> -      - compatible = "google,veyron-jaq-rev5", "google,veyron-jaq-rev4",
> -		     "google,veyron-jaq-rev3", "google,veyron-jaq-rev2",
> -		     "google,veyron-jaq-rev1", "google,veyron-jaq",
> -		     "google,veyron", "rockchip,rk3288";
> -
> -- Google Jerry (Hisense Chromebook C11 and more):
> -    Required root node properties:
> -      - compatible = "google,veyron-jerry-rev7", "google,veyron-jerry-rev6",
> -		     "google,veyron-jerry-rev5", "google,veyron-jerry-rev4",
> -		     "google,veyron-jerry-rev3", "google,veyron-jerry",
> -		     "google,veyron", "rockchip,rk3288";
> -
> -- Google Kevin (Samsung Chromebook Plus):
> -    Required root node properties:
> -      - compatible = "google,kevin-rev15", "google,kevin-rev14",
> -		     "google,kevin-rev13", "google,kevin-rev12",
> -		     "google,kevin-rev11", "google,kevin-rev10",
> -		     "google,kevin-rev9", "google,kevin-rev8",
> -		     "google,kevin-rev7", "google,kevin-rev6",
> -		     "google,kevin", "google,gru", "rockchip,rk3399";
> -
> -- Google Mickey (Asus Chromebit CS10):
> -    Required root node properties:
> -      - compatible = "google,veyron-mickey-rev8", "google,veyron-mickey-rev7",
> -		     "google,veyron-mickey-rev6", "google,veyron-mickey-rev5",
> -		     "google,veyron-mickey-rev4", "google,veyron-mickey-rev3",
> -		     "google,veyron-mickey-rev2", "google,veyron-mickey-rev1",
> -		     "google,veyron-mickey-rev0", "google,veyron-mickey",
> -		     "google,veyron", "rockchip,rk3288";
> -
> -- Google Minnie (Asus Chromebook Flip C100P):
> -    Required root node properties:
> -      - compatible = "google,veyron-minnie-rev4", "google,veyron-minnie-rev3",
> -		     "google,veyron-minnie-rev2", "google,veyron-minnie-rev1",
> -		     "google,veyron-minnie-rev0", "google,veyron-minnie",
> -		     "google,veyron", "rockchip,rk3288";
> -
> -- Google Pinky (dev-board):
> -    Required root node properties:
> -      - compatible = "google,veyron-pinky-rev2", "google,veyron-pinky",
> -		     "google,veyron", "rockchip,rk3288";
> -
> -- Google Speedy (Asus C201 Chromebook):
> -    Required root node properties:
> -      - compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
> -		     "google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
> -		     "google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
> -		     "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
> -		     "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
> -
> -- mqmaker MiQi:
> -    Required root node properties:
> -      - compatible = "mqmaker,miqi", "rockchip,rk3288";
> -
> -- Phytec phyCORE-RK3288: Rapid Development Kit
> -    Required root node properties:
> -     - compatible = "phytec,rk3288-pcm-947", "phytec,rk3288-phycore-som", "rockchip,rk3288";
> -
> -- Pine64 Rock64 board:
> -    Required root node properties:
> -    - compatible = "pine64,rock64", "rockchip,rk3328";
> -
> -- Pine64 RockPro64 board:
> -    Required root node properties:
> -    - compatible = "pine64,rockpro64", "rockchip,rk3399";
> -
> -- Rockchip PX3 Evaluation board:
> -    Required root node properties:
> -      - compatible = "rockchip,px3-evb", "rockchip,px3", "rockchip,rk3188";
> -
> -- Rockchip PX5 Evaluation board:
> -    Required root node properties:
> -      - compatible = "rockchip,px5-evb", "rockchip,px5", "rockchip,rk3368";
> -
> -- Rockchip PX30 Evaluation board:
> -    Required root node properties:
> -      - compatible = "rockchip,px30-evb", "rockchip,px30";
> -
> -- Rockchip RV1108 Evaluation board
> -    Required root node properties:
> -      - compatible = "rockchip,rv1108-evb", "rockchip,rv1108";
> -
> -- Rockchip RK3368 evb:
> -    Required root node properties:
> -      - compatible = "rockchip,rk3368-evb-act8846", "rockchip,rk3368";
> -
> -- Rockchip R88 board:
> -    Required root node properties:
> -      - compatible = "rockchip,r88", "rockchip,rk3368";
> -
> -- Rockchip RK3228 Evaluation board:
> -    Required root node properties:
> -     - compatible = "rockchip,rk3228-evb", "rockchip,rk3228";
> -
> -- Rockchip RK3229 Evaluation board:
> -     - compatible = "rockchip,rk3229-evb", "rockchip,rk3229";
> -
> -- Rockchip RK3288 Fennec board:
> -    Required root node properties:
> -     - compatible = "rockchip,rk3288-fennec", "rockchip,rk3288";
> -
> -- Rockchip RK3328 evb:
> -    Required root node properties:
> -      - compatible = "rockchip,rk3328-evb", "rockchip,rk3328";
> -
> -- Rockchip RK3399 evb:
> -    Required root node properties:
> -      - compatible = "rockchip,rk3399-evb", "rockchip,rk3399";
> -
> -- Rockchip RK3399 Sapphire board standalone:
> -    Required root node properties:
> -      - compatible = "rockchip,rk3399-sapphire", "rockchip,rk3399";
> -
> -- Rockchip RK3399 Sapphire Excavator board:
> -    Required root node properties:
> -      - compatible = "rockchip,rk3399-sapphire-excavator", "rockchip,rk3399";
> -
> -- Theobroma Systems RK3368-uQ7 Haikou Baseboard:
> -    Required root node properties:
> -      - compatible = "tsd,rk3368-uq7-haikou", "rockchip,rk3368";
> -
> -- Theobroma Systems RK3399-Q7 Haikou Baseboard:
> -    Required root node properties:
> -      - compatible = "tsd,rk3399-q7-haikou", "rockchip,rk3399";
> -
> -- Tronsmart Orion R68 Meta
> -    Required root node properties:
> -      - compatible = "tronsmart,orion-r68-meta", "rockchip,rk3368";
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.yaml b/Documentation/devicetree/bindings/arm/rockchip.yaml
> new file mode 100644
> index 000000000000..36f4bf4b445c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/rockchip.yaml
> @@ -0,0 +1,419 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/rockchip.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip platforms device tree bindings
> +
> +maintainers:
> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +properties:
> +  $nodename:
> +    const: '/'
> +  compatible:
> +    oneOf:
> +
> +      - description: 96boards RK3399 Ficus (ROCK960 Enterprise Edition)
> +        items:
> +          - const: vamrs,ficus
> +          - const: rockchip,rk3399
> +
> +      - description: 96boards RK3399 Rock960 (ROCK960 Consumer Edition)
> +        items:
> +          - const: vamrs,rock960
> +          - const: rockchip,rk3399
> +
> +      - description: Amarula Vyasa RK3288
> +        items:
> +          - const: amarula,vyasa-rk3288
> +          - const: rockchip,rk3288
> +
> +      - description: Asus Tinker board
> +        items:
> +          - const: asus,rk3288-tinker
> +          - const: rockchip,rk3288
> +
> +      - description: Asus Tinker board S
> +        items:
> +          - const: asus,rk3288-tinker-s
> +          - const: rockchip,rk3288
> +
> +      - description: bq Curie 2 tablet
> +        items:
> +          - const: mundoreader,bq-curie2
> +          - const: rockchip,rk3066a
> +
> +      - description: bq Edison 2 Quad-Core tablet
> +        items:
> +          - const: mundoreader,bq-edison2qc
> +          - const: rockchip,rk3188
> +
> +      - description: ChipSPARK PopMetal-RK3288
> +        items:
> +          - const: chipspark,popmetal-rk3288
> +          - const: rockchip,rk3288
> +
> +      - description: ChipSPARK Rayeager PX2
> +        items:
> +          - const: chipspark,rayeager-px2
> +          - const: rockchip,rk3066a
> +
> +      - description: Firefly Firefly-RK3288
> +        items:
> +          - const: firefly,firefly-rk3288
> +          - const: rockchip,rk3288
> +
> +      - description: Firefly Firefly-RK3288 (beta board)
> +        items:
> +          - const: firefly,firefly-rk3288-beta
> +          - const: rockchip,rk3288
> +
> +      - description: Firefly Firefly-RK3288 Reload
> +        items:
> +          - const: firefly,firefly-rk3288-reload
> +          - const: rockchip,rk3288
> +
> +      - description: Firefly Firefly-RK3399
> +        items:
> +          - const: firefly,firefly-rk3399
> +          - const: rockchip,rk3399
> +
> +      - description: Firefly roc-rk3328-cc
> +        items:
> +          - const: firefly,roc-rk3328-cc
> +          - const: rockchip,rk3328
> +
> +      - description: Firefly ROC-RK3399-PC
> +        items:
> +          - const: firefly,roc-rk3399-pc
> +          - const: rockchip,rk3399
> +
> +      - description: GeekBuying GeekBox
> +        items:
> +          - const: geekbuying,geekbox
> +          - const: rockchip,rk3368
> +
> +      - description: Google Bob (Asus Chromebook Flip C101PA)
> +        items:
> +          - const: google,bob-rev13
> +          - const: google,bob-rev12
> +          - const: google,bob-rev11
> +          - const: google,bob-rev10
> +          - const: google,bob-rev9
> +          - const: google,bob-rev8
> +          - const: google,bob-rev7
> +          - const: google,bob-rev6
> +          - const: google,bob-rev5
> +          - const: google,bob-rev4
> +          - const: google,bob
> +          - const: google,gru
> +          - const: rockchip,rk3399
> +
> +      - description: Google Brain (dev-board)
> +        items:
> +          - const: google,veyron-brain-rev0
> +          - const: google,veyron-brain
> +          - const: google,veyron
> +          - const: rockchip,rk3288
> +
> +      - description: Google Gru (dev-board)
> +        items:
> +          - const: google,gru-rev15
> +          - const: google,gru-rev14
> +          - const: google,gru-rev13
> +          - const: google,gru-rev12
> +          - const: google,gru-rev11
> +          - const: google,gru-rev10
> +          - const: google,gru-rev9
> +          - const: google,gru-rev8
> +          - const: google,gru-rev7
> +          - const: google,gru-rev6
> +          - const: google,gru-rev5
> +          - const: google,gru-rev4
> +          - const: google,gru-rev3
> +          - const: google,gru-rev2
> +          - const: google,gru
> +          - const: rockchip,rk3399
> +
> +      - description: Google Jaq (Haier Chromebook 11 and more)
> +        items:
> +          - const: google,veyron-jaq-rev5
> +          - const: google,veyron-jaq-rev4
> +          - const: google,veyron-jaq-rev3
> +          - const: google,veyron-jaq-rev2
> +          - const: google,veyron-jaq-rev1
> +          - const: google,veyron-jaq
> +          - const: google,veyron
> +          - const: rockchip,rk3288
> +
> +      - description: Google Jerry (Hisense Chromebook C11 and more)
> +        items:
> +          - const: google,veyron-jerry-rev7
> +          - const: google,veyron-jerry-rev6
> +          - const: google,veyron-jerry-rev5
> +          - const: google,veyron-jerry-rev4
> +          - const: google,veyron-jerry-rev3
> +          - const: google,veyron-jerry
> +          - const: google,veyron
> +          - const: rockchip,rk3288
> +
> +      - description: Google Kevin (Samsung Chromebook Plus)
> +        items:
> +          - const: google,kevin-rev15
> +          - const: google,kevin-rev14
> +          - const: google,kevin-rev13
> +          - const: google,kevin-rev12
> +          - const: google,kevin-rev11
> +          - const: google,kevin-rev10
> +          - const: google,kevin-rev9
> +          - const: google,kevin-rev8
> +          - const: google,kevin-rev7
> +          - const: google,kevin-rev6
> +          - const: google,kevin
> +          - const: google,gru
> +          - const: rockchip,rk3399
> +
> +      - description: Google Mickey (Asus Chromebit CS10)
> +        items:
> +          - const: google,veyron-mickey-rev8
> +          - const: google,veyron-mickey-rev7
> +          - const: google,veyron-mickey-rev6
> +          - const: google,veyron-mickey-rev5
> +          - const: google,veyron-mickey-rev4
> +          - const: google,veyron-mickey-rev3
> +          - const: google,veyron-mickey-rev2
> +          - const: google,veyron-mickey-rev1
> +          - const: google,veyron-mickey-rev0
> +          - const: google,veyron-mickey
> +          - const: google,veyron
> +          - const: rockchip,rk3288
> +
> +      - description: Google Minnie (Asus Chromebook Flip C100P)
> +        items:
> +          - const: google,veyron-minnie-rev4
> +          - const: google,veyron-minnie-rev3
> +          - const: google,veyron-minnie-rev2
> +          - const: google,veyron-minnie-rev1
> +          - const: google,veyron-minnie-rev0
> +          - const: google,veyron-minnie
> +          - const: google,veyron
> +          - const: rockchip,rk3288
> +
> +      - description: Google Pinky (dev-board)
> +        items:
> +          - const: google,veyron-pinky-rev2
> +          - const: google,veyron-pinky
> +          - const: google,veyron
> +          - const: rockchip,rk3288
> +
> +      - description: Google Scarlet - Kingdisplay (Acer Chromebook Tab 10)
> +        items:
> +          - const: google,scarlet-rev15-sku7
> +          - const: google,scarlet-rev15
> +          - const: google,scarlet-rev14-sku7
> +          - const: google,scarlet-rev14
> +          - const: google,scarlet-rev13-sku7
> +          - const: google,scarlet-rev13
> +          - const: google,scarlet-rev12-sku7
> +          - const: google,scarlet-rev12
> +          - const: google,scarlet-rev11-sku7
> +          - const: google,scarlet-rev11
> +          - const: google,scarlet-rev10-sku7
> +          - const: google,scarlet-rev10
> +          - const: google,scarlet-rev9-sku7
> +          - const: google,scarlet-rev9
> +          - const: google,scarlet-rev8-sku7
> +          - const: google,scarlet-rev8
> +          - const: google,scarlet-rev7-sku7
> +          - const: google,scarlet-rev7
> +          - const: google,scarlet-rev6-sku7
> +          - const: google,scarlet-rev6
> +          - const: google,scarlet-rev5-sku7
> +          - const: google,scarlet-rev5
> +          - const: google,scarlet-rev4-sku7
> +          - const: google,scarlet-rev4
> +          - const: google,scarlet-rev3-sku7
> +          - const: google,scarlet-rev3
> +          - const: google,scarlet
> +          - const: google,gru
> +          - const: rockchip,rk3399
> +
> +      - description: Google Scarlet - Innolux display (Acer Chromebook Tab 10)
> +        items:
> +          - const: google,scarlet-rev15-sku6
> +          - const: google,scarlet-rev15
> +          - const: google,scarlet-rev14-sku6
> +          - const: google,scarlet-rev14
> +          - const: google,scarlet-rev13-sku6
> +          - const: google,scarlet-rev13
> +          - const: google,scarlet-rev12-sku6
> +          - const: google,scarlet-rev12
> +          - const: google,scarlet-rev11-sku6
> +          - const: google,scarlet-rev11
> +          - const: google,scarlet-rev10-sku6
> +          - const: google,scarlet-rev10
> +          - const: google,scarlet-rev9-sku6
> +          - const: google,scarlet-rev9
> +          - const: google,scarlet-rev8-sku6
> +          - const: google,scarlet-rev8
> +          - const: google,scarlet-rev7-sku6
> +          - const: google,scarlet-rev7
> +          - const: google,scarlet-rev6-sku6
> +          - const: google,scarlet-rev6
> +          - const: google,scarlet-rev5-sku6
> +          - const: google,scarlet-rev5
> +          - const: google,scarlet-rev4-sku6
> +          - const: google,scarlet-rev4
> +          - const: google,scarlet
> +          - const: google,gru
> +          - const: rockchip,rk3399
> +
> +      - description: Google Speedy (Asus C201 Chromebook)
> +        items:
> +          - const: google,veyron-speedy-rev9
> +          - const: google,veyron-speedy-rev8
> +          - const: google,veyron-speedy-rev7
> +          - const: google,veyron-speedy-rev6
> +          - const: google,veyron-speedy-rev5
> +          - const: google,veyron-speedy-rev4
> +          - const: google,veyron-speedy-rev3
> +          - const: google,veyron-speedy-rev2
> +          - const: google,veyron-speedy
> +          - const: google,veyron
> +          - const: rockchip,rk3288
> +
> +      - description: Haoyu MarsBoard RK3066
> +        items:
> +          - const: haoyu,marsboard-rk3066
> +          - const: rockchip,rk3066a
> +
> +      - description: mqmaker MiQi
> +        items:
> +          - const: mqmaker,miqi
> +          - const: rockchip,rk3288
> +
> +      - description: Netxeon R89 board
> +        items:
> +          - const: netxeon,r89
> +          - const: rockchip,rk3288
> +
> +      - description: Phytec phyCORE-RK3288 Rapid Development Kit
> +        items:
> +          - const: phytec,rk3288-pcm-947
> +          - const: phytec,rk3288-phycore-som
> +          - const: rockchip,rk3288
> +
> +      - description: Pine64 Rock64
> +        items:
> +          - const: pine64,rock64
> +          - const: rockchip,rk3328
> +
> +      - description: Pine64 RockPro64
> +        items:
> +          - const: pine64,rockpro64
> +          - const: rockchip,rk3399
> +
> +      - description: Radxa Rock
> +        items:
> +          - const: radxa,rock
> +          - const: rockchip,rk3188
> +
> +      - description: Radxa Rock2 Square
> +        items:
> +          - const: radxa,rock2-square
> +          - const: rockchip,rk3288
> +
> +      - description: Rikomagic MK808 v1
> +        items:
> +          - const: rikomagic,mk808
> +          - const: rockchip,rk3066a
> +
> +      - description: Rockchip Kylin
> +        items:
> +          - const: rockchip,kylin-rk3036
> +          - const: rockchip,rk3036
> +
> +      - description: Rockchip PX3 Evaluation board
> +        items:
> +          - const: rockchip,px3-evb
> +          - const: rockchip,px3
> +          - const: rockchip,rk3188
> +
> +      - description: Rockchip PX30 Evaluation board
> +        items:
> +          - const: rockchip,px30-evb
> +          - const: rockchip,px30
> +
> +      - description: Rockchip PX5 Evaluation board
> +        items:
> +          - const: rockchip,px5-evb
> +          - const: rockchip,px5
> +          - const: rockchip,rk3368
> +
> +      - description: Rockchip R88
> +        items:
> +          - const: rockchip,r88
> +          - const: rockchip,rk3368
> +
> +      - description: Rockchip RK3228 Evaluation board
> +        items:
> +          - const: rockchip,rk3228-evb
> +          - const: rockchip,rk3228
> +
> +      - description: Rockchip RK3288 Fennec
> +        items:
> +          - const: rockchip,rk3288-fennec
> +          - const: rockchip,rk3288
> +
> +      - description: Rockchip RK3229 Evaluation board
> +        items:
> +          - const: rockchip,rk3229-evb
> +          - const: rockchip,rk3229
> +
> +      - description: Rockchip RK3328 Evaluation board
> +        items:
> +          - const: rockchip,rk3328-evb
> +          - const: rockchip,rk3328
> +
> +      - description: Rockchip RK3368 Evaluation board (act8846 pmic)
> +        items:
> +          - const: rockchip,rk3368-evb-act8846
> +          - const: rockchip,rk3368
> +
> +      - description: Rockchip RK3399 Evaluation board
> +        items:
> +          - const: rockchip,rk3399-evb
> +          - const: rockchip,rk3399
> +
> +      - description: Rockchip RK3399 Sapphire standalone
> +        items:
> +          - const: rockchip,rk3399-sapphire
> +          - const: rockchip,rk3399
> +
> +      - description: Rockchip RK3399 Sapphire with Excavator Baseboard
> +        items:
> +          - const: rockchip,rk3399-sapphire-excavator
> +          - const: rockchip,rk3399
> +
> +      - description: Rockchip RV1108 Evaluation board
> +        items:
> +          - const: rockchip,rv1108-evb
> +          - const: rockchip,rv1108
> +
> +      - description: Theobroma Systems RK3368-uQ7 with Haikou baseboard
> +        items:
> +          - const: tsd,rk3368-uq7-haikou
> +          - const: rockchip,rk3368
> +
> +      - description: Theobroma Systems RK3399-Q7 with Haikou baseboard
> +        items:
> +          - const: tsd,rk3399-q7-haikou
> +          - const: rockchip,rk3399
> +
> +      - description: Tronsmart Orion R68 Meta
> +        items:
> +          - const: tronsmart,orion-r68-meta
> +          - const: rockchip,rk3368
> +...
> 





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 01/10] mailbox: Support blocking transfers in atomic context
From: Thierry Reding @ 2018-12-10  9:52 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Devicetree List, Greg KH, mliljeberg, Mikko Perttunen, talho,
	linux-serial, jslaby, linux-tegra, ppessi, Jon Hunter,
	linux-arm-kernel
In-Reply-To: <CABb+yY2jVmzxFR7iqSyMMyE+XZtyHrAPAxM_KMEa7c2zzpSwCA@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 10309 bytes --]

On Sat, Dec 08, 2018 at 11:39:05AM +0530, Jassi Brar wrote:
> On Fri, Dec 7, 2018 at 5:02 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > On Thu, Dec 06, 2018 at 11:56:25PM -0600, Jassi Brar wrote:
> > > On Thu, Nov 29, 2018 at 9:23 AM Thierry Reding <thierry.reding@gmail.com> wrote:
> > > >
> > > > On Wed, Nov 28, 2018 at 11:23:36PM -0600, Jassi Brar wrote:
> > > > > On Wed, Nov 28, 2018 at 3:43 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> > > > > >
> > > > > >
> > > > > > On 12/11/2018 15:18, Thierry Reding wrote:
> > > > > > > From: Thierry Reding <treding@nvidia.com>
> > > > > > >
> > > > > > > The mailbox framework supports blocking transfers via completions for
> > > > > > > clients that can sleep. In order to support blocking transfers in cases
> > > > > > > where the transmission is not permitted to sleep, add a new ->flush()
> > > > > > > callback that controller drivers can implement to busy loop until the
> > > > > > > transmission has been completed. This will automatically be called when
> > > > > > > available and interrupts are disabled for clients that request blocking
> > > > > > > transfers.
> > > > > > >
> > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > > > > > > ---
> > > > > > >  drivers/mailbox/mailbox.c          | 8 ++++++++
> > > > > > >  include/linux/mailbox_controller.h | 4 ++++
> > > > > > >  2 files changed, 12 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> > > > > > > index 674b35f402f5..0eaf21259874 100644
> > > > > > > --- a/drivers/mailbox/mailbox.c
> > > > > > > +++ b/drivers/mailbox/mailbox.c
> > > > > > > @@ -267,6 +267,14 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
> > > > > > >               unsigned long wait;
> > > > > > >               int ret;
> > > > > > >
> > > > > > > +             if (irqs_disabled() && chan->mbox->ops->flush) {
> > > > > > > +                     ret = chan->mbox->ops->flush(chan, chan->cl->tx_tout);
> > > > > > > +                     if (ret < 0)
> > > > > > > +                             tx_tick(chan, ret);
> > > > > > > +
> > > > > > > +                     return ret;
> > > > > > > +             }
> > > > > >
> > > > > > It seems to me that if mbox_send_message() is called from an atomic
> > > > > > context AND tx_block is true, then if 'flush' is not populated this
> > > > > > should be an error condition as we do not wish to call
> > > > > > wait_for_completion from an atomic context.
> > > > > >
> > > > > > I understand that there is some debate about adding such flush support,
> > > > > > but irrespective of the above change, it seems to me that if the
> > > > > > mbox_send_message() can be called from an atomic context (which it
> > > > > > appears to), then it should be detecting if someone is trying to do so
> > > > > > with 'tx_block' set as this should be an error.
> > > > > >
> > > > > Layers within kernel space have to trust each other. A badly written
> > > > > client can break the consumer in so many ways, we can not catch every
> > > > > possibility.
> > > > >
> > > > > > Furthermore, if the underlying mailbox driver can support sending a
> > > > > > message from an atomic context and busy wait until it is done, surely
> > > > > > the mailbox framework should provide a means to support this?
> > > > > >
> > > > > Being able to put the message on bus in atomic context is a feature -
> > > > > which we do support. But busy-waiting in a loop is not a feature, and
> > > > > we don't want to encourage that.
> > > >
> > > > I agree that in generally busy-waiting is a bad idea and shouldn't be
> > > > encouraged. However, I also think that an exception proves the rule. If
> > > > you look at the console drivers in drivers/tty/serial, all of them will
> > > > busy loop prior to or after sending a character. This is pretty much
> > > > part of the API and as such busy-looping is very much a feature.
> > > >
> > > > The reason why this is done is because on one hand we have an atomic
> > > > context and on the other hand we want to make sure that all characters
> > > > actually make it to the console when we print them.
> > > >
> > > > As an example how this can break, I've taken your suggestion to
> > > > implement a producer/consumer mode in the TCU driver where the console
> > > > write function will just stash characters into a circular buffer and a
> > > > work queue will then use mbox_send_message() to drain the circular
> > > > buffer. While this does work on the surface, I was able to concern both
> > > > of the issues that I was concerned about: 1) it is easy to overflow the
> > > > circular buffer by just dumping enough data at once to the console and
> > > > 2) when a crash happens, everything in the kernel stops, including the
> > > > consumer workqueue that is supposed to drain the circular buffer and
> > > > flush messages to the TCU. The result is that, in my particular case,
> > > > the boot log will just stop at a random place in the middle of messages
> > > > from much earlier in the boot because the TCU hasn't caught up yet and
> > > > there's a lot of characters still in the circular buffer.
> > > >
> > > > Now, 1) can be mitigated by increasing the circular buffer size. A value
> > > > that seems to give reliably good results in 2 << CONFIG_LOG_BUF_SHIFT.
> > > >
> > > Yes please.
> >
> > As I explained elsewhere, I actually went and implemented this. But
> > given the nature of buffering, this ends up making the TCU completely
> > useless as a console because in case of a crash, the system will stop
> > working with a large number of characters still stuck in the buffer.
> > And that's especially bad in case of a crash because those last
> > characters that get stuck in the buffer are the most relevant ones
> > because they contain the stack dump.
> >
> I don't question the utility of TCU. I just wonder if mailbox api
> should provide a backdoor for atomic busy-wait in order to support a
> sub-optimal hw+fw design.
> 
> > > > I thought that I could also mitigate 2) by busy looping in the TCU driver,
> > > > but it turns out that that doesn't work. The reason is that since we are
> > > > in atomic context with all interrupts disabled, the mailbox won't ever
> > > > consume any new characters, so the read pointer in the circular buffer
> > > > won't increment, leaving me with no condition upon which to loop that
> > > > would work.
> > > >
> > > So you want to be able to rely on an emulated console (running on a
> > > totally different subsystem) to dump development-phase early-boot
> > > logs? At the cost of legitimizing busy looping in atomic context - one
> > > random driver messing up the api for ever. Maybe you could have the
> > > ring buffer in some shmem and only pass the number of valid characters
> > > in it, to the remote?
> >
> > First of all, this is not about development-phase early-boot messages.
> > We're talking about the system console here. This is what everyone will
> > want to be using when developing on this device. Sure at some point you
> > may end up with a system that works and you can have the console on the
> > network or an attached display or whatever, but even then you may still
> > want to attach to the console if you ever run into issues where the
> > system doesn't come up.
> >
> > Secondly, I don't understand why you think this is an emulated console.
> >
> It is emulated/virtual because Linux doesn't put characters on a
> physical bus. The data is simply handed forward to a remote entity.

Okay, I understand. However, from a kernel point of view there's no
difference between the TCU and any physical UART. Once the data is
handed to the TCU TX mailbox, it's out of control of the TCU driver.
Whether there's a real UART behind that or some remote processor that
reads the data isn't really relevant.

> > Lastly, I don't understand why you think we're messing up the API here.
> > The proposal in this series doesn't even change any of the API, but it
> > makes it aware of the state of interrupts internally so that it can do
> > the right thing depending on the call stack. The other proposal, in v3,
> > is more explicit in that it adds new API to flush the mailbox. The new
> > API is completely optional and I even offered to document it as being
> > discouraged because it involves busy looping. At the same time it does
> > solve a real problem and it doesn't impact any existing mailbox drivers
> > nor any of their users (because it is completely opt-in).
> >
> The 'flush' api is going to have no use other than implement
> busy-waits. I am afraid people are simply going to take it easy and
> copy the busy-waits from the test/verification codes.
> "discouraging" seldom works because the developer comes with the
> failproof excuse "the h/w doesn't provide any other way". Frankly I
> would like to push back on such designs.

I certainly approve of that stance.

However, I'd like to reiterate that the TCU design does in fact support
"another way". If you look at the mailbox driver implementation (in the
drivers/mailbox/tegra-hsp.c driver), you'll see that it does support an
interrupt driven mode. After you had reviewed Mikko's earliest proposal
that was indeed implementing busy looping exclusively we both set out
to properly implement interrupt support. This took quite a while to do
because of some gaps in the documentation, but we eventually got it to
work properly. And then it was discovered that it was all a waste of
effort because the console driver couldn't make use of it anyway. Well,
I should say "waste of effort" because I'm still happy that the proper
implementation exists and we can use it under the right circumstances.

So, at least in this particular case, it is not the hardware or firmware
design that's flawed or was taking any shortcuts. It's really just the
particular use-case of the console that doesn't allow us to make use of
the right implementation, which is why we have to resort to the inferior
method of busy-looping.

>  Anyways, let us add the new 'flush' api.

Great, thanks!

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v6 4/4] arm64: KVM: Enable support for :G/:H perf event modifiers
From: Andrew Murray @ 2018-12-10  9:45 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Suzuki K Poulose, kvmarm, linux-arm-kernel, Julien Thierry
In-Reply-To: <1544435159-54781-1-git-send-email-andrew.murray@arm.com>

Enable/disable event counters as appropriate when entering and exiting
the guest to enable support for guest or host only event counting.

For both VHE and non-VHE we switch the counters between host/guest at
EL2. EL2 is filtered out by the PMU when we are using the :G modifier.

The PMU may be on when we change which counters are enabled however
we avoid adding an isb as we instead rely on existing context
synchronisation events: the isb in kvm_arm_vhe_guest_exit for VHE and
the eret from the hvc in kvm_call_hyp.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kvm/hyp/switch.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index d496ef5..e505cad 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -373,6 +373,32 @@ static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+static bool __hyp_text __pmu_switch_to_guest(struct kvm_cpu_context *host_ctxt)
+{
+	u32 clr = host_ctxt->events_host & ~host_ctxt->events_guest;
+	u32 set = host_ctxt->events_guest & ~host_ctxt->events_host;
+
+	if (clr)
+		write_sysreg(clr, pmcntenclr_el0);
+
+	if (set)
+		write_sysreg(set, pmcntenset_el0);
+
+	return (clr || set);
+}
+
+static void __hyp_text __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
+{
+	u32 clr = host_ctxt->events_guest & ~host_ctxt->events_host;
+	u32 set = host_ctxt->events_host & ~host_ctxt->events_guest;
+
+	if (clr)
+		write_sysreg(clr, pmcntenclr_el0);
+
+	if (set)
+		write_sysreg(set, pmcntenset_el0);
+}
+
 /*
  * Return true when we were able to fixup the guest exit and should return to
  * the guest, false when we should restore the host state and return to the
@@ -488,12 +514,15 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;
 	struct kvm_cpu_context *guest_ctxt;
+	bool pmu_switch_needed;
 	u64 exit_code;
 
 	host_ctxt = vcpu->arch.host_cpu_context;
 	host_ctxt->__hyp_running_vcpu = vcpu;
 	guest_ctxt = &vcpu->arch.ctxt;
 
+	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
+
 	sysreg_save_host_state_vhe(host_ctxt);
 
 	__activate_traps(vcpu);
@@ -524,6 +553,9 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu)
 
 	__debug_switch_to_host(vcpu);
 
+	if (pmu_switch_needed)
+		__pmu_switch_to_host(host_ctxt);
+
 	return exit_code;
 }
 
@@ -532,6 +564,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;
 	struct kvm_cpu_context *guest_ctxt;
+	bool pmu_switch_needed;
 	u64 exit_code;
 
 	vcpu = kern_hyp_va(vcpu);
@@ -540,6 +573,8 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	host_ctxt->__hyp_running_vcpu = vcpu;
 	guest_ctxt = &vcpu->arch.ctxt;
 
+	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
+
 	__sysreg_save_state_nvhe(host_ctxt);
 
 	__activate_traps(vcpu);
@@ -586,6 +621,9 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	 */
 	__debug_switch_to_host(vcpu);
 
+	if (pmu_switch_needed)
+		__pmu_switch_to_host(host_ctxt);
+
 	return exit_code;
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 1/4] arm64: arm_pmu: remove unnecessary isb instruction
From: Andrew Murray @ 2018-12-10  9:45 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Suzuki K Poulose, kvmarm, linux-arm-kernel, Julien Thierry
In-Reply-To: <1544435159-54781-1-git-send-email-andrew.murray@arm.com>

The armv8pmu_enable_event_counter function issues an isb instruction
after enabling a pair of counters - this doesn't provide any value
and is inconsistent with the armv8pmu_disable_event_counter.

In any case armv8pmu_enable_event_counter is always called with the
PMU stopped. Starting the PMU with armv8pmu_start results in an isb
instruction being issued prior to writing to PMCR_EL0.

Let's remove the unnecessary isb instruction.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/kernel/perf_event.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index 8e38d52..de564ae 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -652,7 +652,6 @@ static inline void armv8pmu_enable_event_counter(struct perf_event *event)
 	armv8pmu_enable_counter(idx);
 	if (armv8pmu_event_is_chained(event))
 		armv8pmu_enable_counter(idx - 1);
-	isb();
 }
 
 static inline int armv8pmu_disable_counter(int idx)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 0/4] arm64: Support perf event modifiers :G and :H
From: Andrew Murray @ 2018-12-10  9:45 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Suzuki K Poulose, kvmarm, linux-arm-kernel, Julien Thierry

This patchset provides support for perf event modifiers :G and :H which
allows for filtering of PMU events between host and guests when used
with KVM.

As the underlying hardware cannot distinguish between guest and host
context, the performance counters must be stopped and started upon
entry/exit to the guest. This is performed at EL2 in a way that
minimizes overhead and improves accuracy of recording events that only
occur in the requested context.

This has been tested with VHE and non-VHE kernels with a KVM guest.

Changes from v5:

 - Tweak logic in use of kvm_set_pmu_events

Changes from v4:

 - Prevent unnecessary write_sysreg calls by improving
   __pmu_switch_to_xxx logic.

Changes from v3:

 - Remove confusing _only suffix from bitfields in kvm_cpu_context
 - Remove unnecessary condition when clearing event bits in disable
 - Simplify API of KVM accessors
 - Prevent unnecessary setting of pmcnten when guest/host events are
   the same.

Changes from v2:

 - Ensured that exclude_kernel works for guest
 - Removed unnecessary exclusion of EL2 with exclude_host on !VHE
 - Renamed kvm_clr_set_host_pmu_events to reflect args order
 - Added additional information to isb patch

Changes from v1:

 - Removed unnecessary exclusion of EL1 with exclude_guest on VHE
 - Removed unnecessary isb from existing perf_event.c driver
 - Folded perf_event.c patches together
 - Added additional information to last patch commit message

Andrew Murray (4):
  arm64: arm_pmu: remove unnecessary isb instruction
  arm64: KVM: add accessors to track guest/host only counters
  arm64: arm_pmu: Add support for exclude_host/exclude_guest attributes
  arm64: KVM: Enable support for :G/:H perf event modifiers

 arch/arm64/include/asm/kvm_host.h | 24 ++++++++++++++++++
 arch/arm64/kernel/perf_event.c    | 52 +++++++++++++++++++++++++++++++++------
 arch/arm64/kvm/hyp/switch.c       | 38 ++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 8 deletions(-)

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v6 2/4] arm64: KVM: add accessors to track guest/host only counters
From: Andrew Murray @ 2018-12-10  9:45 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Suzuki K Poulose, kvmarm, linux-arm-kernel, Julien Thierry
In-Reply-To: <1544435159-54781-1-git-send-email-andrew.murray@arm.com>

In order to effeciently enable/disable guest/host only perf counters
at guest entry/exit we add bitfields to kvm_cpu_context for guest and
host events as well as accessors for updating them.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/include/asm/kvm_host.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 1550192..800c87b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -203,6 +203,8 @@ struct kvm_cpu_context {
 	};
 
 	struct kvm_vcpu *__hyp_running_vcpu;
+	u32 events_host;
+	u32 events_guest;
 };
 
 typedef struct kvm_cpu_context kvm_cpu_context_t;
@@ -467,11 +469,33 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_ctxsync_fp(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu);
 
+#define KVM_PMU_EVENTS_HOST	1
+#define KVM_PMU_EVENTS_GUEST	2
+
 #ifdef CONFIG_KVM /* Avoid conflicts with core headers if CONFIG_KVM=n */
 static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 {
 	return kvm_arch_vcpu_run_map_fp(vcpu);
 }
+static inline void kvm_set_pmu_events(u32 set, int flags)
+{
+	kvm_cpu_context_t *ctx = this_cpu_ptr(&kvm_host_cpu_state);
+
+	if (flags & KVM_PMU_EVENTS_HOST)
+		ctx->events_host |= set;
+	if (flags & KVM_PMU_EVENTS_GUEST)
+		ctx->events_guest |= set;
+}
+static inline void kvm_clr_pmu_events(u32 clr)
+{
+	kvm_cpu_context_t *ctx = this_cpu_ptr(&kvm_host_cpu_state);
+
+	ctx->events_host &= ~clr;
+	ctx->events_guest &= ~clr;
+}
+#else
+static inline void kvm_set_pmu_events(u32 set, int flags) {}
+static inline void kvm_clr_pmu_events(u32 clr) {}
 #endif
 
 static inline void kvm_arm_vhe_guest_enter(void)
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 3/4] arm64: arm_pmu: Add support for exclude_host/exclude_guest attributes
From: Andrew Murray @ 2018-12-10  9:45 UTC (permalink / raw)
  To: Christoffer Dall, Marc Zyngier, Catalin Marinas, Will Deacon,
	Mark Rutland
  Cc: Suzuki K Poulose, kvmarm, linux-arm-kernel, Julien Thierry
In-Reply-To: <1544435159-54781-1-git-send-email-andrew.murray@arm.com>

Add support for the :G and :H attributes in perf by handling the
exclude_host/exclude_guest event attributes.

We notify KVM of counters that we wish to be enabled or disabled on
guest entry/exit and thus defer from starting or stopping :G events
as per the events exclude_host attribute.

With both VHE and non-VHE we switch the counters between host/guest
at EL2. We are able to eliminate counters counting host events on
the boundaries of guest entry/exit when using :G by filtering out
EL2 for exclude_host. However when using :H unless exclude_hv is set
on non-VHE then there is a small blackout window at the guest
entry/exit where host events are not captured.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/kernel/perf_event.c | 51 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index de564ae..4a3c73d 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -26,6 +26,7 @@
 
 #include <linux/acpi.h>
 #include <linux/clocksource.h>
+#include <linux/kvm_host.h>
 #include <linux/of.h>
 #include <linux/perf/arm_pmu.h>
 #include <linux/platform_device.h>
@@ -647,11 +648,26 @@ static inline int armv8pmu_enable_counter(int idx)
 
 static inline void armv8pmu_enable_event_counter(struct perf_event *event)
 {
+	struct perf_event_attr *attr = &event->attr;
 	int idx = event->hw.idx;
+	int flags = 0;
+	u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
 
-	armv8pmu_enable_counter(idx);
 	if (armv8pmu_event_is_chained(event))
-		armv8pmu_enable_counter(idx - 1);
+		counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
+
+	if (!attr->exclude_host)
+		flags |= KVM_PMU_EVENTS_HOST;
+	if (!attr->exclude_guest)
+		flags |= KVM_PMU_EVENTS_GUEST;
+
+	kvm_set_pmu_events(counter_bits, flags);
+
+	if (!attr->exclude_host) {
+		armv8pmu_enable_counter(idx);
+		if (armv8pmu_event_is_chained(event))
+			armv8pmu_enable_counter(idx - 1);
+	}
 }
 
 static inline int armv8pmu_disable_counter(int idx)
@@ -664,11 +680,20 @@ static inline int armv8pmu_disable_counter(int idx)
 static inline void armv8pmu_disable_event_counter(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
+	struct perf_event_attr *attr = &event->attr;
 	int idx = hwc->idx;
+	u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
 
 	if (armv8pmu_event_is_chained(event))
-		armv8pmu_disable_counter(idx - 1);
-	armv8pmu_disable_counter(idx);
+		counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
+
+	kvm_clr_pmu_events(counter_bits);
+
+	if (!attr->exclude_host) {
+		if (armv8pmu_event_is_chained(event))
+			armv8pmu_disable_counter(idx - 1);
+		armv8pmu_disable_counter(idx);
+	}
 }
 
 static inline int armv8pmu_enable_intens(int idx)
@@ -943,16 +968,25 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
 	 * Therefore we ignore exclude_hv in this configuration, since
 	 * there's no hypervisor to sample anyway. This is consistent
 	 * with other architectures (x86 and Power).
+	 *
+	 * To eliminate counting host events on the boundaries of
+	 * guest entry/exit we ensure EL2 is not included in hyp mode
+	 * with !exclude_host.
 	 */
 	if (is_kernel_in_hyp_mode()) {
-		if (!attr->exclude_kernel)
+		if (!attr->exclude_kernel && !attr->exclude_host)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
 	} else {
-		if (attr->exclude_kernel)
-			config_base |= ARMV8_PMU_EXCLUDE_EL1;
 		if (!attr->exclude_hv)
 			config_base |= ARMV8_PMU_INCLUDE_EL2;
 	}
+
+	/*
+	 * Filter out !VHE kernels and guest kernels
+	 */
+	if (attr->exclude_kernel)
+		config_base |= ARMV8_PMU_EXCLUDE_EL1;
+
 	if (attr->exclude_user)
 		config_base |= ARMV8_PMU_EXCLUDE_EL0;
 
@@ -976,6 +1010,9 @@ static void armv8pmu_reset(void *info)
 		armv8pmu_disable_intens(idx);
 	}
 
+	/* Clear the counters we flip at guest entry/exit */
+	kvm_clr_pmu_events(U32_MAX);
+
 	/*
 	 * Initialize & Reset PMNC. Request overflow interrupt for
 	 * 64 bit cycle counter but cheat in armv8pmu_write_counter().
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH 01/19] dt-bindings: clock: tegra124-dfll: Update DFLL binding for PWM regulator
From: Jon Hunter @ 2018-12-10  9:44 UTC (permalink / raw)
  To: Joseph Lo, Thierry Reding, Peter De Schrijver
  Cc: linux-tegra, devicetree, linux-clk, linux-arm-kernel
In-Reply-To: <433ed5cf-70ce-1d03-530f-406b2cb16e07@nvidia.com>


On 10/12/2018 09:31, Joseph Lo wrote:
> On 12/10/18 4:59 PM, Jon Hunter wrote:
>>
>> On 10/12/2018 08:49, Joseph Lo wrote:
>>> Hi Jon,
>>>
>>> Thanks for reviewing this series.
>>>
>>> On 12/7/18 9:41 PM, Jon Hunter wrote:
>>>>
>>>> On 04/12/2018 09:25, Joseph Lo wrote:
>>>>> From: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>>>
>>>>> Add new properties to configure the DFLL PWM regulator support. Also
>>>>> add an example and make the I2C clock only required when I2C
>>>>> support is
>>>>> used.
>>>>>
>>>>> Cc: devicetree@vger.kernel.org
>>>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>>>> ---
>>>>>    .../bindings/clock/nvidia,tegra124-dfll.txt   | 73
>>>>> ++++++++++++++++++-
>>>>>    1 file changed, 71 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>>> b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>>> index dff236f524a7..8c97600d2bad 100644
>>>>> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>>> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>>> @@ -8,7 +8,6 @@ the fast CPU cluster. It consists of a free-running
>>>>> voltage controlled
>>>>>    oscillator connected to the CPU voltage rail (VDD_CPU), and a
>>>>> closed loop
>>>>>    control module that will automatically adjust the VDD_CPU
>>>>> voltage by
>>>>>    communicating with an off-chip PMIC either via an I2C bus or via
>>>>> PWM signals.
>>>>> -Currently only the I2C mode is supported by these bindings.
>>>>>      Required properties:
>>>>>    - compatible : should be "nvidia,tegra124-dfll"
>>>>> @@ -45,10 +44,28 @@ Required properties for the control loop
>>>>> parameters:
>>>>>    Optional properties for the control loop parameters:
>>>>>    - nvidia,cg-scale: Boolean value, see the field
>>>>> DFLL_PARAMS_CG_SCALE in the TRM.
>>>>>    +Optional properties for mode selection:
>>>>> +- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
>>>>> +
>>>>>    Required properties for I2C mode:
>>>>>    - nvidia,i2c-fs-rate: I2C transfer rate, if using full speed mode.
>>>>>    -Example:
>>>>> +Required properties for PWM mode:
>>>>> +- nvidia,pwm-period: period of PWM square wave in microseconds.
>>>>> +- nvidia,init-uv: Regulator voltage in micro volts when PWM control
>>>>> is disabled.
>>>>
>>>> Maybe consider 'pwm-inactive-voltage-microvolt'.
>>> Ah, I think I need to refine the description here. It should be
>>> something like below.
>>>   - nvidia,pwm-init-microvolt : Regulator voltage in micro volts when
>>> PWM
>>> control is initialized
>>>
>>> This is the initial voltage that when we just initialize the DFLL
>>> hardware for PWM output. And before we switch the CPU clock from PLLX to
>>> DFLL, we will enable DFLL hardware in closed loop mode which will aplly
>>> the DVFS table that was calculated from CVB table.
>>>
>>> The original description maybe make you think that it's the working
>>> voltage when it's under open-loop mode. But it's not. Sorry.
>>>
>>> When we working on open-loop mode which will switch to low voltage range
>>> which also follows the DVFS table. Not this one.
>>
>> OK, but I am still not sure what this voltage is. I mean that I
>> understand it is the initial voltage, but how exactly do we define this
>> number? Where does it come from, how is this determined?
> IIRC, this is the same output voltage that bootloader configured with
> proper PLLX rate. So before DFLL handling the CPU clock, that is the
> output voltage. We configure the same when DFLL HW is just initialized.
> 
> 1 volt here is pretty safe when CPU clock switched from bootloader to
> kernel at 6.912MHz@pllx.

OK, now I understand. Seems a little fragile, but maybe there is no
better alternative here.

Please describe what this voltage is in the binding doc, so if anyone
happened to modify their bootloader to run at a different speed that it
is stated that this voltage should be high enough to support initial
boot frequency.

>>
>>>>
>>>>> +- nvidia,align-offset-uv: Regulator voltage in micro volts when PWM
>>>>> control is
>>>>> +              enabled and PWM output is low.
>>>>
>>>> Would this be considered the minimum pwm active voltage?
>>> This would be used for minimum voltage for LUT table, which is the table
>>> that PMIC can output. The real minimum voltage in PWM mode still depends
>>> on the CVB table.
>>>
>>> So maybe change this one to 'nvidia,pwm-offset-uv'.
>>
>> So is this the min supported by the PMIC? Maybe the name should reflect
>> that because the above name does not reflect this. Furthermore, if this
>> is a min then maybe the name should use 'min' as opposed to 'offset'.
>> for example, 'nvidia,pwm-pmic-min-microvolts'.
>>
>> Does this need to be described in DT, can it not be queried from the
>> PMIC?
>>
> Yes, either way needs to go with DT. The very initial patchset for
> DFLL-PWM support, which introduced a redundant pwm-dfll driver. And with
> the driver, we can describe the PWM vdd-cpu regulator in DT with the
> voltage table that the PMIC can output. But NAKed by reviewer.
> 
> Okay, 'nvidia,pwm-pmic-min-microvolts' looks fine. Thanks.
> 
> [1]: https://patchwork.ozlabs.org/patch/613524/

Thierry, what are your thoughts here? I guess you had asked initially
why we needed a phandle to the PMIC. Seems we need to comprehend the min
voltage supported by the PMIC for creating the LUT.

Cheers
Jon

-- 
nvpublic

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
From: Yogesh Narayan Gaur @ 2018-12-10  9:41 UTC (permalink / raw)
  To: Schrempf Frieder, linux-mtd@lists.infradead.org,
	boris.brezillon@bootlin.com, marek.vasut@gmail.com,
	broonie@kernel.org, linux-spi@vger.kernel.org,
	devicetree@vger.kernel.org
  Cc: mark.rutland@arm.com, robh@kernel.org,
	linux-kernel@vger.kernel.org, computersforpeace@gmail.com,
	shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <eb8a31d8-2ba6-f27c-addf-545c77921b77@kontron.de>

Hi Frieder,

Thanks for the review. Please find my comments inline.

> -----Original Message-----
> From: Schrempf Frieder [mailto:frieder.schrempf@kontron.de]
> Sent: Thursday, December 6, 2018 2:53 PM
> To: Yogesh Narayan Gaur <yogeshnarayan.gaur@nxp.com>; linux-
> mtd@lists.infradead.org; boris.brezillon@bootlin.com; marek.vasut@gmail.com;
> broonie@kernel.org; linux-spi@vger.kernel.org; devicetree@vger.kernel.org
> Cc: robh@kernel.org; mark.rutland@arm.com; shawnguo@kernel.org; linux-
> arm-kernel@lists.infradead.org; computersforpeace@gmail.com; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v5 1/5] spi: spi-mem: Add driver for NXP FlexSPI controller
> 
> Hi Yogesh,
> 
> I've had a closer look at your v5. See my comments below.
> 
> On 16.11.18 12:13, Yogesh Narayan Gaur wrote:
> > - Add driver for NXP FlexSPI host controller
> >
> > (0) What is the FlexSPI controller?
> >   FlexSPI is a flexsible SPI host controller which supports two SPI
> >   channels and up to 4 external devices. Each channel supports
> >   Single/Dual/Quad/Octal mode data transfer (1/2/4/8 bidirectional
> >   data lines) i.e. FlexSPI acts as an interface to external devices,
> >   maximum 4, each with up to 8 bidirectional data lines.
> >
> >   It uses new SPI memory interface of the SPI framework to issue
> >   flash memory operations to up to four connected flash
> >   devices (2 buses with 2 CS each).
> >
> > (1) Tested this driver with the mtd_debug and JFFS2 filesystem utility
> >   on NXP LX2160ARDB and LX2160AQDS targets.
> >   LX2160ARDB is having two NOR slave device connected on single bus A
> >   i.e. A0 and A1 (CS0 and CS1).
> >   LX2160AQDS is having two NOR slave device connected on separate buses
> >   one flash on A0 and second on B1 i.e. (CS0 and CS3).
> >   Verified this driver on following SPI NOR flashes:
> >      Micron, mt35xu512ab, [Read - 1 bit mode]
> >      Cypress, s25fl512s, [Read - 1/2/4 bit mode]
> >
> > Signed-off-by: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
> > ---
> > Changes for v5:
> > - Rebase on top of v4.20-rc2
> > - Modified fspi_readl_poll_tout() as per review comments
> > - Arrange header file in alphabetical order
> > - Removed usage of read()/write() function callback pointer
> > - Add support for 1 and 2 byte address length
> > - Change Frieder e-mail to new e-mail address Changes for v4:
> > - Incorporate Boris review comments
> >    * Use readl_poll_timeout() instead of busy looping.
> >    * Re-define register masking as per comment.
> >    * Drop fspi_devtype enum.
> > Changes for v3:
> > - Added endianness flag in platform specific structure instead of DTS.
> > - Modified nxp_fspi_read_ahb(), removed remapping code.
> > - Added Boris and Frieder as Author and provided reference of
> > spi-fsl-qspi.c Changes for v2:
> > - Incorporated Boris review comments.
> > - Remove dependency of driver over connected flash device size.
> > - Modified the logic to select requested CS.
> > - Remove SPI-Octal Macros.
> >
> >   drivers/spi/Kconfig        |   10 +
> >   drivers/spi/Makefile       |    1 +
> >   drivers/spi/spi-nxp-fspi.c | 1145
> ++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 1156 insertions(+)
> >   create mode 100644 drivers/spi/spi-nxp-fspi.c
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> > 7d3a5c9..36630a1 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -259,6 +259,16 @@ config SPI_FSL_LPSPI
> >   	help
> >   	  This enables Freescale i.MX LPSPI controllers in master mode.
> >
> > +config SPI_NXP_FLEXSPI
> > +	tristate "NXP Flex SPI controller"
> > +	depends on ARCH_LAYERSCAPE || HAS_IOMEM
> > +	help
> > +	  This enables support for the Flex SPI controller in master mode.
> > +	  Up to four slave devices can be connected on two buses with two
> > +	  chipselects each.
> > +	  This controller does not support generic SPI messages and only
> > +	  supports the high-level SPI memory interface.
> > +
> >   config SPI_GPIO
> >   	tristate "GPIO-based bitbanging SPI Master"
> >   	depends on GPIOLIB || COMPILE_TEST
> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index
> > 3575205..55fec5c 100644
> > --- a/drivers/spi/Makefile
> > +++ b/drivers/spi/Makefile
> > @@ -60,6 +60,7 @@ obj-$(CONFIG_SPI_MPC52xx)		+= spi-
> mpc52xx.o
> >   obj-$(CONFIG_SPI_MT65XX)                += spi-mt65xx.o
> >   obj-$(CONFIG_SPI_MXS)			+= spi-mxs.o
> >   obj-$(CONFIG_SPI_NUC900)		+= spi-nuc900.o
> > +obj-$(CONFIG_SPI_NXP_FLEXSPI)		+= spi-nxp-fspi.o
> >   obj-$(CONFIG_SPI_OC_TINY)		+= spi-oc-tiny.o
> >   spi-octeon-objs				:= spi-cavium.o spi-cavium-
> octeon.o
> >   obj-$(CONFIG_SPI_OCTEON)		+= spi-octeon.o
> > diff --git a/drivers/spi/spi-nxp-fspi.c b/drivers/spi/spi-nxp-fspi.c
> > new file mode 100644 index 0000000..a35013b
> > --- /dev/null
> > +++ b/drivers/spi/spi-nxp-fspi.c
> > @@ -0,0 +1,1145 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/*
> > + * NXP FlexSPI(FSPI) controller driver.
> > + *
> > + * Copyright 2018 NXP.
> > + *
> > + * FlexSPI is a flexsible SPI host controller which supports two SPI
> > + * channels and up to 4 external devices. Each channel supports
> > + * Single/Dual/Quad/Octal mode data transfer (1/2/4/8 bidirectional
> > + * data lines).
> > + *
> > + * FlexSPI controller is driven by the LUT(Look-up Table) registers
> > + * LUT registers are a look-up-table for sequences of instructions.
> > + * A valid sequence consists of four LUT registers.
> > + * Maximum 32 LUT sequences can be programmed simultaneously.
> > + *
> > + * LUTs are being created at run-time based on the commands passed
> > + * from the spi-mem framework, thus using single LUT index.
> > + *
> > + * Software triggered Flash read/write access by IP Bus.
> > + *
> > + * Memory mapped read access by AHB Bus.
> > + *
> > + * Based on SPI MEM interface and spi-fsl-qspi.c driver.
> > + *
> > + * Author:
> > + *     Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
> > + *     Boris Brezillion <boris.brezillon@bootlin.com>
> > + *     Frieder Schrempf <frieder.schrempf@kontron.de>
> > + */
> > +
> > +#include <linux/bitops.h>
> > +#include <linux/clk.h>
> > +#include <linux/completion.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/errno.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_qos.h>
> > +#include <linux/sizes.h>
> > +
> > +#include <linux/spi/spi.h>
> > +#include <linux/spi/spi-mem.h>
> > +
> > +/*
> > + * The driver only uses one single LUT entry, that is updated on
> > + * each call of exec_op(). Index 0 is preset at boot with a basic
> > + * read operation, so let's use the last entry (31).
> > + */
> > +#define	SEQID_LUT			31
> > +
> > +/* Registers used by the driver */
> > +#define FSPI_MCR0			0x00
> > +#define FSPI_MCR0_AHB_TIMEOUT(x)	((x) << 24)
> > +#define FSPI_MCR0_IP_TIMEOUT(x)		((x) << 16)
> > +#define FSPI_MCR0_LEARN_EN		BIT(15)
> > +#define FSPI_MCR0_SCRFRUN_EN		BIT(14)
> > +#define FSPI_MCR0_OCTCOMB_EN		BIT(13)
> > +#define FSPI_MCR0_DOZE_EN		BIT(12)
> > +#define FSPI_MCR0_HSEN			BIT(11)
> > +#define FSPI_MCR0_SERCLKDIV		BIT(8)
> > +#define FSPI_MCR0_ATDF_EN		BIT(7)
> > +#define FSPI_MCR0_ARDF_EN		BIT(6)
> > +#define FSPI_MCR0_RXCLKSRC(x)		((x) << 4)
> > +#define FSPI_MCR0_END_CFG(x)		((x) << 2)
> > +#define FSPI_MCR0_MDIS			BIT(1)
> > +#define FSPI_MCR0_SWRST			BIT(0)
> > +
> > +#define FSPI_MCR1			0x04
> > +#define FSPI_MCR1_SEQ_TIMEOUT(x)	((x) << 16)
> > +#define FSPI_MCR1_AHB_TIMEOUT(x)	(x)
> > +
> > +#define FSPI_MCR2			0x08
> > +#define FSPI_MCR2_IDLE_WAIT(x)		((x) << 24)
> > +#define FSPI_MCR2_SAMEDEVICEEN		BIT(15)
> > +#define FSPI_MCR2_CLRLRPHS		BIT(14)
> > +#define FSPI_MCR2_ABRDATSZ		BIT(8)
> > +#define FSPI_MCR2_ABRLEARN		BIT(7)
> > +#define FSPI_MCR2_ABR_READ		BIT(6)
> > +#define FSPI_MCR2_ABRWRITE		BIT(5)
> > +#define FSPI_MCR2_ABRDUMMY		BIT(4)
> > +#define FSPI_MCR2_ABR_MODE		BIT(3)
> > +#define FSPI_MCR2_ABRCADDR		BIT(2)
> > +#define FSPI_MCR2_ABRRADDR		BIT(1)
> > +#define FSPI_MCR2_ABR_CMD		BIT(0)
> > +
> > +#define FSPI_AHBCR			0x0c
> > +#define FSPI_AHBCR_RDADDROPT		BIT(6)
> > +#define FSPI_AHBCR_PREF_EN		BIT(5)
> > +#define FSPI_AHBCR_BUFF_EN		BIT(4)
> > +#define FSPI_AHBCR_CACH_EN		BIT(3)
> > +#define FSPI_AHBCR_CLRTXBUF		BIT(2)
> > +#define FSPI_AHBCR_CLRRXBUF		BIT(1)
> > +#define FSPI_AHBCR_PAR_EN		BIT(0)
> > +
> > +#define FSPI_INTEN			0x10
> > +#define FSPI_INTEN_SCLKSBWR		BIT(9)
> > +#define FSPI_INTEN_SCLKSBRD		BIT(8)
> > +#define FSPI_INTEN_DATALRNFL		BIT(7)
> > +#define FSPI_INTEN_IPTXWE		BIT(6)
> > +#define FSPI_INTEN_IPRXWA		BIT(5)
> > +#define FSPI_INTEN_AHBCMDERR		BIT(4)
> > +#define FSPI_INTEN_IPCMDERR		BIT(3)
> > +#define FSPI_INTEN_AHBCMDGE		BIT(2)
> > +#define FSPI_INTEN_IPCMDGE		BIT(1)
> > +#define FSPI_INTEN_IPCMDDONE		BIT(0)
> > +
> > +#define FSPI_INTR			0x14
> > +#define FSPI_INTR_SCLKSBWR		BIT(9)
> > +#define FSPI_INTR_SCLKSBRD		BIT(8)
> > +#define FSPI_INTR_DATALRNFL		BIT(7)
> > +#define FSPI_INTR_IPTXWE		BIT(6)
> > +#define FSPI_INTR_IPRXWA		BIT(5)
> > +#define FSPI_INTR_AHBCMDERR		BIT(4)
> > +#define FSPI_INTR_IPCMDERR		BIT(3)
> > +#define FSPI_INTR_AHBCMDGE		BIT(2)
> > +#define FSPI_INTR_IPCMDGE		BIT(1)
> > +#define FSPI_INTR_IPCMDDONE		BIT(0)
> > +
> > +#define FSPI_LUTKEY			0x18
> > +#define FSPI_LUTKEY_VALUE		0x5AF05AF0
> > +
> > +#define FSPI_LCKCR			0x1C
> > +
> > +#define FSPI_LCKER_LOCK			0x1
> > +#define FSPI_LCKER_UNLOCK		0x2
> > +
> > +#define FSPI_BUFXCR_INVALID_MSTRID	0xE
> > +#define FSPI_AHBRX_BUF0CR0		0x20
> > +#define FSPI_AHBRX_BUF1CR0		0x24
> > +#define FSPI_AHBRX_BUF2CR0		0x28
> > +#define FSPI_AHBRX_BUF3CR0		0x2C
> > +#define FSPI_AHBRX_BUF4CR0		0x30
> > +#define FSPI_AHBRX_BUF5CR0		0x34
> > +#define FSPI_AHBRX_BUF6CR0		0x38
> > +#define FSPI_AHBRX_BUF7CR0		0x3C
> > +#define FSPI_AHBRXBUF0CR7_PREF		BIT(31)
> > +
> > +#define FSPI_AHBRX_BUF0CR1		0x40
> > +#define FSPI_AHBRX_BUF1CR1		0x44
> > +#define FSPI_AHBRX_BUF2CR1		0x48
> > +#define FSPI_AHBRX_BUF3CR1		0x4C
> > +#define FSPI_AHBRX_BUF4CR1		0x50
> > +#define FSPI_AHBRX_BUF5CR1		0x54
> > +#define FSPI_AHBRX_BUF6CR1		0x58
> > +#define FSPI_AHBRX_BUF7CR1		0x5C
> > +
> > +#define FSPI_FLSHA1CR0			0x60
> > +#define FSPI_FLSHA2CR0			0x64
> > +#define FSPI_FLSHB1CR0			0x68
> > +#define FSPI_FLSHB2CR0			0x6C
> > +#define FSPI_FLSHXCR0_SZ_KB		10
> > +#define FSPI_FLSHXCR0_SZ(x)		((x) >> FSPI_FLSHXCR0_SZ_KB)
> > +
> > +#define FSPI_FLSHA1CR1			0x70
> > +#define FSPI_FLSHA2CR1			0x74
> > +#define FSPI_FLSHB1CR1			0x78
> > +#define FSPI_FLSHB2CR1			0x7C
> > +#define FSPI_FLSHXCR1_CSINTR(x)		((x) << 16)
> > +#define FSPI_FLSHXCR1_CAS(x)		((x) << 11)
> > +#define FSPI_FLSHXCR1_WA		BIT(10)
> > +#define FSPI_FLSHXCR1_TCSH(x)		((x) << 5)
> > +#define FSPI_FLSHXCR1_TCSS(x)		(x)
> > +
> > +#define FSPI_FLSHA1CR2			0x80
> > +#define FSPI_FLSHA2CR2			0x84
> > +#define FSPI_FLSHB1CR2			0x88
> > +#define FSPI_FLSHB2CR2			0x8C
> > +#define FSPI_FLSHXCR2_CLRINSP		BIT(24)
> > +#define FSPI_FLSHXCR2_AWRWAIT		BIT(16)
> > +#define FSPI_FLSHXCR2_AWRSEQN_SHIFT	13
> > +#define FSPI_FLSHXCR2_AWRSEQI_SHIFT	8
> > +#define FSPI_FLSHXCR2_ARDSEQN_SHIFT	5
> > +#define FSPI_FLSHXCR2_ARDSEQI_SHIFT	0
> > +
> > +#define FSPI_IPCR0			0xA0
> > +
> > +#define FSPI_IPCR1			0xA4
> > +#define FSPI_IPCR1_IPAREN		BIT(31)
> > +#define FSPI_IPCR1_SEQNUM_SHIFT		24
> > +#define FSPI_IPCR1_SEQID_SHIFT		16
> > +#define FSPI_IPCR1_IDATSZ(x)		(x)
> > +
> > +#define FSPI_IPCMD			0xB0
> > +#define FSPI_IPCMD_TRG			BIT(0)
> > +
> > +#define FSPI_DLPR			0xB4
> > +
> > +#define FSPI_IPRXFCR			0xB8
> > +#define FSPI_IPRXFCR_CLR		BIT(0)
> > +#define FSPI_IPRXFCR_DMA_EN		BIT(1)
> > +#define FSPI_IPRXFCR_WMRK(x)		((x) << 2)
> > +
> > +#define FSPI_IPTXFCR			0xBC
> > +#define FSPI_IPTXFCR_CLR		BIT(0)
> > +#define FSPI_IPTXFCR_DMA_EN		BIT(1)
> > +#define FSPI_IPTXFCR_WMRK(x)		((x) << 2)
> > +
> > +#define FSPI_DLLACR			0xC0
> > +#define FSPI_DLLACR_OVRDEN		BIT(8)
> > +
> > +#define FSPI_DLLBCR			0xC4
> > +#define FSPI_DLLBCR_OVRDEN		BIT(8)
> > +
> > +#define FSPI_STS0			0xE0
> > +#define FSPI_STS0_DLPHB(x)		((x) << 8)
> > +#define FSPI_STS0_DLPHA(x)		((x) << 4)
> > +#define FSPI_STS0_CMD_SRC(x)		((x) << 2)
> > +#define FSPI_STS0_ARB_IDLE		BIT(1)
> > +#define FSPI_STS0_SEQ_IDLE		BIT(0)
> > +
> > +#define FSPI_STS1			0xE4
> > +#define FSPI_STS1_IP_ERRCD(x)		((x) << 24)
> > +#define FSPI_STS1_IP_ERRID(x)		((x) << 16)
> > +#define FSPI_STS1_AHB_ERRCD(x)		((x) << 8)
> > +#define FSPI_STS1_AHB_ERRID(x)		(x)
> > +
> > +#define FSPI_AHBSPNST			0xEC
> > +#define FSPI_AHBSPNST_DATLFT(x)		((x) << 16)
> > +#define FSPI_AHBSPNST_BUFID(x)		((x) << 1)
> > +#define FSPI_AHBSPNST_ACTIVE		BIT(0)
> > +
> > +#define FSPI_IPRXFSTS			0xF0
> > +#define FSPI_IPRXFSTS_RDCNTR(x)		((x) << 16)
> > +#define FSPI_IPRXFSTS_FILL(x)		(x)
> > +
> > +#define FSPI_IPTXFSTS			0xF4
> > +#define FSPI_IPTXFSTS_WRCNTR(x)		((x) << 16)
> > +#define FSPI_IPTXFSTS_FILL(x)		(x)
> > +
> > +#define FSPI_RFDR			0x100
> > +#define FSPI_TFDR			0x180
> > +
> > +#define FSPI_LUT_BASE			0x200
> > +#define FSPI_LUT_OFFSET			(SEQID_LUT * 4 * 4)
> > +#define FSPI_LUT_REG(idx) \
> > +	(FSPI_LUT_BASE + FSPI_LUT_OFFSET + (idx) * 4)
> > +
> > +/* register map end */
> > +
> > +/* Instruction set for the LUT register. */
> > +#define LUT_STOP			0x00
> > +#define LUT_CMD				0x01
> > +#define LUT_ADDR			0x02
> > +#define LUT_CADDR_SDR			0x03
> > +#define LUT_MODE			0x04
> > +#define LUT_MODE2			0x05
> > +#define LUT_MODE4			0x06
> > +#define LUT_MODE8			0x07
> > +#define LUT_NXP_WRITE			0x08
> > +#define LUT_NXP_READ			0x09
> > +#define LUT_LEARN_SDR			0x0A
> > +#define LUT_DATSZ_SDR			0x0B
> > +#define LUT_DUMMY			0x0C
> > +#define LUT_DUMMY_RWDS_SDR		0x0D
> > +#define LUT_JMP_ON_CS			0x1F
> > +#define LUT_CMD_DDR			0x21
> > +#define LUT_ADDR_DDR			0x22
> > +#define LUT_CADDR_DDR			0x23
> > +#define LUT_MODE_DDR			0x24
> > +#define LUT_MODE2_DDR			0x25
> > +#define LUT_MODE4_DDR			0x26
> > +#define LUT_MODE8_DDR			0x27
> > +#define LUT_WRITE_DDR			0x28
> > +#define LUT_READ_DDR			0x29
> > +#define LUT_LEARN_DDR			0x2A
> > +#define LUT_DATSZ_DDR			0x2B
> > +#define LUT_DUMMY_DDR			0x2C
> > +#define LUT_DUMMY_RWDS_DDR		0x2D
> > +
> > +/*
> > + * Calculate number of required PAD bits for LUT register.
> > + *
> > + * The pad stands for the number of IO lines [0:7].
> > + * For example, the octal read needs eight IO lines,
> > + * so you should use LUT_PAD(8). This macro
> > + * returns 3 i.e. use eight (2^3) IP lines for read.
> > + */
> > +#define LUT_PAD(x) (fls(x) - 1)
> > +
> > +/*
> > + * Macro for constructing the LUT entries with the following
> > + * register layout:
> > + *
> > + *  ---------------------------------------------------
> > + *  | INSTR1 | PAD1 | OPRND1 | INSTR0 | PAD0 | OPRND0 |
> > + *  ---------------------------------------------------
> > + */
> > +#define PAD_SHIFT		8
> > +#define INSTR_SHIFT		10
> > +#define OPRND_SHIFT		16
> > +
> > +/* Macros for constructing the LUT register. */
> > +#define LUT_DEF(idx, ins, pad, opr)			  \
> > +	((((ins) << INSTR_SHIFT) | ((pad) << PAD_SHIFT) | \
> > +	(opr)) << (((idx) % 2) * OPRND_SHIFT))
> > +
> > +/* Operands for the LUT register. */
> > +#define ADDR8BIT		0x08
> > +#define ADDR16BIT		0x10
> > +#define ADDR24BIT		0x18
> > +#define ADDR32BIT		0x20
> 
> You can drop these ADDRXBIT definitions, see below...
> 
Ok, would drop in next version.

> > +
> > +#define POLL_TOUT		5000
> > +#define NXP_FSPI_MAX_CHIPSELECT		4
> > +
> > +struct nxp_fspi_devtype_data {
> > +	unsigned int rxfifo;
> > +	unsigned int txfifo;
> > +	unsigned int ahb_buf_size;
> > +	unsigned int quirks;
> > +	bool little_endian;
> > +};
> > +
> > +static const struct nxp_fspi_devtype_data lx2160a_data = {
> > +	.rxfifo = SZ_512,       /* (64  * 64 bits)  */
> > +	.txfifo = SZ_1K,        /* (128 * 64 bits)  */
> > +	.ahb_buf_size = SZ_2K,  /* (256 * 64 bits)  */
> > +	.quirks = 0,
> > +	.little_endian = true,  /* little-endian    */
> > +};
> > +
> > +struct nxp_fspi {
> > +	void __iomem *iobase;
> > +	void __iomem *ahb_addr;
> > +	u32 memmap_phy;
> > +	u32 memmap_phy_size;
> > +	struct clk *clk, *clk_en;
> > +	struct device *dev;
> > +	struct completion c;
> > +	const struct nxp_fspi_devtype_data *devtype_data;
> > +	struct mutex lock;
> > +	struct pm_qos_request pm_qos_req;
> > +	int selected;
> > +};
> > +
> > +/*
> > + * R/W functions for big- or little-endian registers:
> > + * The FSPI controller's endianness is independent of
> > + * the CPU core's endianness. So far, although the CPU
> > + * core is little-endian the FSPI controller can use
> > + * big-endian or little-endian.
> > + */
> > +static void fspi_writel(struct nxp_fspi *f, u32 val, void __iomem
> > +*addr) {
> > +	if (f->devtype_data->little_endian)
> > +		iowrite32(val, addr);
> > +	else
> > +		iowrite32be(val, addr);
> > +}
> > +
> > +static u32 fspi_readl(struct nxp_fspi *f, void __iomem *addr) {
> > +	if (f->devtype_data->little_endian)
> > +		return ioread32(addr);
> > +	else
> > +		return ioread32be(addr);
> > +}
> > +
> > +static irqreturn_t nxp_fspi_irq_handler(int irq, void *dev_id) {
> > +	struct nxp_fspi *f = dev_id;
> > +	u32 reg;
> > +
> > +	/* clear interrupt */
> > +	reg = fspi_readl(f, f->iobase + FSPI_INTR);
> > +	fspi_writel(f, FSPI_INTR_IPCMDDONE, f->iobase + FSPI_INTR);
> > +
> > +	if (reg & FSPI_INTR_IPCMDDONE)
> > +		complete(&f->c);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int nxp_fspi_check_buswidth(struct nxp_fspi *f, u8 width) {
> > +	switch (width) {
> > +	case 1:
> > +	case 2:
> > +	case 4:
> > +	case 8:
> > +		return 0;
> > +	}
> > +
> > +	return -ENOTSUPP;
> > +}
> > +
> > +static bool nxp_fspi_supports_op(struct spi_mem *mem,
> > +				 const struct spi_mem_op *op)
> > +{
> > +	struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> > +	int ret;
> > +
> > +	ret = nxp_fspi_check_buswidth(f, op->cmd.buswidth);
> > +
> > +	if (op->addr.nbytes)
> > +		ret |= nxp_fspi_check_buswidth(f, op->addr.buswidth);
> > +
> > +	if (op->dummy.nbytes)
> > +		ret |= nxp_fspi_check_buswidth(f, op->dummy.buswidth);
> > +
> > +	if (op->data.nbytes)
> > +		ret |= nxp_fspi_check_buswidth(f, op->data.buswidth);
> > +
> > +	if (ret)
> > +		return false;
> > +
> > +	/*
> > +	 * The number of instructions needed for the op, needs
> > +	 * to fit into a single LUT entry.
> > +	 */
> > +	if (op->addr.nbytes +
> > +	   (op->dummy.nbytes ? 1:0) +
> > +	   (op->data.nbytes ? 1:0) > 6)
> > +		return false;
> > +
> > +	/* Max 64 dummy clock cycles supported */
> > +	if (op->dummy.buswidth &&
> > +	    (op->dummy.nbytes * 8 / op->dummy.buswidth > 64))
> > +		return false;
> > +
> > +	/* Max data length, check controller limits and alignment */
> > +	if (op->data.dir == SPI_MEM_DATA_IN &&
> > +	    (op->data.nbytes > f->devtype_data->ahb_buf_size ||
> > +	     (op->data.nbytes > f->devtype_data->rxfifo - 4 &&
> > +	      !IS_ALIGNED(op->data.nbytes, 8))))
> > +		return false;
> > +
> > +	if (op->data.dir == SPI_MEM_DATA_OUT &&
> > +	    op->data.nbytes > f->devtype_data->txfifo)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> > +/* Instead of busy looping invoke readl_poll_timeout functionality.
> > +*/ static int fspi_readl_poll_tout(struct nxp_fspi *f, void __iomem *base,
> > +				u32 mask, u32 delay_us,
> > +				u32 timeout_us, bool condition)
> > +{
> > +	u32 reg;
> > +
> > +	if (!f->devtype_data->little_endian)
> > +		mask = (u32)cpu_to_be32(mask);
> > +
> > +	if (condition)
> > +		return readl_poll_timeout(base, reg, (reg & mask),
> > +					  delay_us, timeout_us);
> > +	else
> > +		return readl_poll_timeout(base, reg, !(reg & mask),
> > +					  delay_us, timeout_us);
> 
> I would rather use a local variable to store the condition:
> 
> bool c = condition ? (reg & mask):!(reg & mask);
> 
With these type of usage getting below warning messages.
 
drivers/spi/spi-nxp-fspi.c: In function ‘fspi_readl_poll_tout.isra.10.constprop’:
drivers/spi/spi-nxp-fspi.c:446:21: warning: ‘reg’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  bool cn = c ? (reg & mask) : !(reg & mask);

If assign value to reg = 0xffffffff then timeout is start getting hit for False case and if assign value 0 then start getting timeout hit for true case.

I would rather not try to modify this function.

> return readl_poll_timeout(base, reg, c, delay_us, timeout_us);
> 
> > +}
> > +
> > +/*
> > + * If the slave device content being changed by Write/Erase, need to
> > + * invalidate the AHB buffer. This can be achieved by doing the reset
> > + * of controller after setting MCR0[SWRESET] bit.
> > + */
> > +static inline void nxp_fspi_invalid(struct nxp_fspi *f) {
> > +	u32 reg;
> > +	int ret;
> > +
> > +	reg = fspi_readl(f, f->iobase + FSPI_MCR0);
> > +	fspi_writel(f, reg | FSPI_MCR0_SWRST, f->iobase + FSPI_MCR0);
> > +
> > +	/* w1c register, wait unit clear */
> > +	ret = fspi_readl_poll_tout(f, f->iobase + FSPI_MCR0,
> > +				   FSPI_MCR0_SWRST, 0, POLL_TOUT, false);
> > +	WARN_ON(ret);
> > +}
> > +
> > +static void nxp_fspi_prepare_lut(struct nxp_fspi *f,
> > +				 const struct spi_mem_op *op)
> > +{
> > +	void __iomem *base = f->iobase;
> > +	u32 lutval[4] = {};
> > +	int lutidx = 1, i;
> > +
> > +	/* cmd */
> > +	lutval[0] |= LUT_DEF(0, LUT_CMD, LUT_PAD(op->cmd.buswidth),
> > +			     op->cmd.opcode);
> > +
> > +	/* addr bus width */
> > +	if (op->addr.nbytes) {
> > +		u32 addrlen = 0;
> > +
> > +		switch (op->addr.nbytes) {
> > +		case 1:
> > +			addrlen = ADDR8BIT;
> > +			break;
> > +		case 2:
> > +			addrlen = ADDR16BIT;
> > +			break;
> > +		case 3:
> > +			addrlen = ADDR24BIT;
> > +			break;
> > +		case 4:
> > +			addrlen = ADDR32BIT;
> > +			break;
> > +		default:
> > +			dev_err(f->dev, "In-correct address length\n");
> > +			return;
> > +		}
> 
> You don't need to validate op->addr.nbytes here, this is already done in
> nxp_fspi_supports_op().

Yes, I need to validate op->addr.nbytes else LUT would going to be programmed for 0 addrlen.
I have checked this on the target.

> 
> > +
> > +		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
> > +					      LUT_PAD(op->addr.buswidth),
> > +					      addrlen);
> 
> You can also just remove the whole switch statement above and use this:
> 
> lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_ADDR,
> 			      LUT_PAD(op->addr.buswidth),
> 			      op->addr.nbytes * 8);
> 
Ok, would include in next version.

> > +		lutidx++;
> > +	}
> > +
> > +	/* dummy bytes, if needed */
> > +	if (op->dummy.nbytes) {
> > +		lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_DUMMY,
> > +		/*
> > +		 * Due to FlexSPI controller limitation number of PAD for
> dummy
> > +		 * buswidth needs to be programmed as equal to data buswidth.
> > +		 */
> > +					      LUT_PAD(op->data.buswidth),
> > +					      op->dummy.nbytes * 8 /
> > +					      op->dummy.buswidth);
> > +		lutidx++;
> > +	}
> > +
> > +	/* read/write data bytes */
> > +	if (op->data.nbytes) {
> > +		lutval[lutidx / 2] |= LUT_DEF(lutidx,
> > +					      op->data.dir ==
> SPI_MEM_DATA_IN ?
> > +					      LUT_NXP_READ : LUT_NXP_WRITE,
> > +					      LUT_PAD(op->data.buswidth),
> > +					      0);
> > +		lutidx++;
> > +	}
> > +
> > +	/* stop condition. */
> > +	lutval[lutidx / 2] |= LUT_DEF(lutidx, LUT_STOP, 0, 0);
> > +
> > +	/* unlock LUT */
> > +	fspi_writel(f, FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
> > +	fspi_writel(f, FSPI_LCKER_UNLOCK, f->iobase + FSPI_LCKCR);
> > +
> > +	/* fill LUT */
> > +	for (i = 0; i < ARRAY_SIZE(lutval); i++)
> > +		fspi_writel(f, lutval[i], base + FSPI_LUT_REG(i));
> > +
> > +	dev_dbg(f->dev, "CMD[%x] lutval[0:%x \t 1:%x \t 2:%x \t 3:%x]\n",
> > +		op->cmd.opcode, lutval[0], lutval[1], lutval[2], lutval[3]);
> > +
> > +	/* lock LUT */
> > +	fspi_writel(f, FSPI_LUTKEY_VALUE, f->iobase + FSPI_LUTKEY);
> > +	fspi_writel(f, FSPI_LCKER_LOCK, f->iobase + FSPI_LCKCR); }
> > +
> > +static int nxp_fspi_clk_prep_enable(struct nxp_fspi *f) {
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(f->clk_en);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = clk_prepare_enable(f->clk);
> > +	if (ret) {
> > +		clk_disable_unprepare(f->clk_en);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void nxp_fspi_clk_disable_unprep(struct nxp_fspi *f) {
> > +	clk_disable_unprepare(f->clk);
> > +	clk_disable_unprepare(f->clk_en);
> > +}
> > +
> > +/*
> > + * In FlexSPI controller, flash access is based on value of
> > +FSPI_FLSHXXCR0
> > + * register and start base address of the slave device.
> > + *
> > + *							    (Higher address)
> > + *				--------    <-- FLSHB2CR0
> > + *				|  B2  |
> > + *				|      |
> > + *	B2 start address -->	--------    <-- FLSHB1CR0
> > + *				|  B1  |
> > + *				|      |
> > + *	B1 start address -->	--------    <-- FLSHA2CR0
> > + *				|  A2  |
> > + *				|      |
> > + *	A2 start address -->	--------    <-- FLSHA1CR0
> > + *				|  A1  |
> > + *				|      |
> > + *	A1 start address -->	--------		    (Lower address)
> > + *
> > + *
> > + * Start base address defines the starting address range for given CS
> > +and
> > + * FSPI_FLSHXXCR0 defines the size of the slave device connected at given CS.
> > + *
> > + * But, different targets are having different combinations of number
> > +of CS,
> > + * some targets only have single CS or two CS covering controller's
> > +full
> > + * memory mapped space area.
> > + * Thus, implementation is being done as independent of the size and
> > +number
> > + * of the connected slave device.
> > + * Assign controller memory mapped space size as the size to the
> > +connected
> > + * slave device.
> > + * Mark FLSHxxCR0 as zero initially and then assign value only to the
> > +selected
> > + * chip-select Flash configuration register.
> > + *
> > + * For e.g. to access CS2 (B1), FLSHB1CR0 register would be equal to
> > +the
> > + * memory mapped size of the controller.
> > + * Value for rest of the CS FLSHxxCR0 register would be zero.
> > + *
> > + */
> > +static void nxp_fspi_select_mem(struct nxp_fspi *f, struct spi_device
> > +*spi) {
> > +	unsigned long rate = spi->max_speed_hz;
> > +	int ret;
> > +	uint64_t size_kb;
> > +
> > +	/*
> > +	 * Return, if previously selected slave device is same as current
> > +	 * requested slave device.
> > +	 */
> > +	if (f->selected == spi->chip_select)
> > +		return;
> > +
> > +	/* Reset FLSHxxCR0 registers */
> > +	fspi_writel(f, 0, f->iobase + FSPI_FLSHA1CR0);
> > +	fspi_writel(f, 0, f->iobase + FSPI_FLSHA2CR0);
> > +	fspi_writel(f, 0, f->iobase + FSPI_FLSHB1CR0);
> > +	fspi_writel(f, 0, f->iobase + FSPI_FLSHB2CR0);
> > +
> > +	/* Assign controller memory mapped space as size, KBytes, of flash. */
> > +	size_kb = FSPI_FLSHXCR0_SZ(f->memmap_phy_size);
> 
Above description of this function, explains the reason for using memmap_phy_size.
This is not the arbitrary size, but the memory mapped size being assigned to the controller.

> You are still using memory of arbitrary size (memmap_phy_size) for mapping the
> flash. Why not use the same approach as in the QSPI driver and just map
> ahb_buf_size until we implement the dirmap API?
The approach which being used in QSPI driver didn't work here, I have tried with that.
In QSPI driver, while preparing LUT we are assigning read/write address in the LUT preparation and have to for some unknown hack have to provide macro for LUT_MODE instead of LUT_ADDR.
But this thing didn't work for FlexSPI.
I discussed with HW IP owner and they suggested only to use LUT_ADDR for specifying the address length of the command i.e. 3-byte or 4-byte address command (NOR) or 1-2 byte address command for NAND.

Thus, in LUT preparation we have assigned only the base address.
Now if I have assigned ahb_buf_size to FSPI_FLSHXXCR0 register then for read/write data beyond limit of ahb_buf_size offset I get data corruption.

Thus, for generic approach have assigned FSPI_FLSHXXCR0 equal to the memory mapped size to the controller. This would also not going to depend on the number of CS present on the target.

> You are already aligning the AHB reads for this in nxp_fspi_adjust_op_size().
> 
Yes, max read data size can be ahb_buf_size. Thus we need to check max read size with ahb_buf_size.

> > +
> > +	switch (spi->chip_select) {
> > +	case 0:
> > +		fspi_writel(f, size_kb, f->iobase + FSPI_FLSHA1CR0);
> > +		break;
> > +	case 1:
> > +		fspi_writel(f, size_kb, f->iobase + FSPI_FLSHA2CR0);
> > +		break;
> > +	case 2:
> > +		fspi_writel(f, size_kb, f->iobase + FSPI_FLSHB1CR0);
> > +		break;
> > +	case 3:
> > +		fspi_writel(f, size_kb, f->iobase + FSPI_FLSHB2CR0);
> > +		break;
> > +	default:
> > +		dev_err(f->dev, "In-correct CS provided\n");
> > +		return;
> 
> You don't need to validate spi->chip_select here. This should never be invalid
> and if it is, something is really wrong and your check won't help.
Ok, would remove in next version.

> 
> > +	}
> > +
> > +	dev_dbg(f->dev, "Slave device [CS:%x] selected\n",
> > +spi->chip_select);
> > +
> > +	nxp_fspi_clk_disable_unprep(f);
> > +
> > +	ret = clk_set_rate(f->clk, rate);
> > +	if (ret)
> > +		return;
> > +
> > +	ret = nxp_fspi_clk_prep_enable(f);
> > +	if (ret)
> > +		return;
> 
> Missing newline line here.
Ok

> 
> > +	f->selected = spi->chip_select;
> > +}
> > +
> > +static void nxp_fspi_read_ahb(struct nxp_fspi *f, const struct
> > +spi_mem_op *op) {
> > +	u32 len = op->data.nbytes;
> > +
> > +	/* Read out the data directly from the AHB buffer. */
> > +	memcpy_fromio(op->data.buf.in, (f->ahb_addr + op->addr.val), len); }
> > +
> > +static void nxp_fspi_fill_txfifo(struct nxp_fspi *f,
> > +				 const struct spi_mem_op *op)
> > +{
> > +	void __iomem *base = f->iobase;
> > +	int i, j, ret;
> > +	int size, tmp_size, wm_size;
> > +	u32 data = 0;
> > +	u32 *txbuf = (u32 *) op->data.buf.out;
> > +
> > +	/* clear the TX FIFO. */
> > +	fspi_writel(f, FSPI_IPTXFCR_CLR, base + FSPI_IPTXFCR);
> > +
> > +	/* Default value of water mark level is 8 bytes. */
> > +	wm_size = 8;
> > +	size = op->data.nbytes / wm_size;
> > +	for (i = 0; i < size; i++) {
> > +		/* Wait for TXFIFO empty */
> > +		ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > +					   FSPI_INTR_IPTXWE, 0,
> > +					   POLL_TOUT, true);
> > +		WARN_ON(ret);
> > +
> > +		j = 0;
> > +		tmp_size = wm_size;
> > +		while (tmp_size > 0) {
> > +			data = 0;
> > +			memcpy(&data, txbuf, 4);
> > +			fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > +			tmp_size -= 4;
> > +			j++;
> > +			txbuf += 1;
> > +		}
> > +		fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > +	}
> > +
> > +	size = op->data.nbytes % wm_size;
> > +	if (size) {
> > +		/* Wait for TXFIFO empty */
> > +		ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > +					   FSPI_INTR_IPTXWE, 0,
> > +					   POLL_TOUT, true);
> > +		WARN_ON(ret);
> > +
> > +		j = 0;
> > +		tmp_size = 0;
> > +		while (size > 0) {
> > +			data = 0;
> > +			tmp_size = (size < 4) ? size : 4;
> > +			memcpy(&data, txbuf, tmp_size);
> > +			fspi_writel(f, data, base + FSPI_TFDR + j * 4);
> > +			size -= tmp_size;
> > +			j++;
> > +			txbuf += 1;
> > +		}
> > +		fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR);
> > +	}
> 
> All these nested loops to fill the TX buffer and also the ones below to read the
> RX buffer look much more complicated than they should really be. Can you try to
> make this more readable?
Yes
> 
> Maybe something like this would work:
> 
> for (i = 0; i < ALIGN_DOWN(op->data.nbytes, 8); i += 8) {
> 	/* Wait for TXFIFO empty */
> 	ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> 				   FSPI_INTR_IPTXWE, 0,
> 				   POLL_TOUT, true);
> 
> 	fspi_writel(f, op->data.buf.out + i, base + FSPI_TFDR);
> 	fspi_writel(f, op->data.buf.out + i + 4, base + FSPI_TFDR + 4);
> 	fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR); }
With this above 2 lines we are hardcoding it for read/write with watermark size as 8 bytes.
Watermark size can be variable and depends on the value of IPRXFCR/IPTXFCR register with default value as 8 bytes
Thus, I would still prefer to use the internal for loop instead of 2 fspi_writel(...) for FSPI_TFDR and FSPI_TFDR + 4 register write commands.

> 
> if (i < op->data.nbytes) {
> 	u32 data = 0;
> 	int j;
> 	/* Wait for TXFIFO empty */
> 	ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> 				   FSPI_INTR_IPTXWE, 0,
> 				   POLL_TOUT, true);
> 
> 	for (j = 0; j < ALIGN(op->data.nbytes - i, 4); j += 4) {
> 		memcpy(&data, op->data.buf.out + i + j, 4);
> 		fspi_writel(f, data, base + FSPI_TFDR + j);
> 	}
> 
> 	fspi_writel(f, FSPI_INTR_IPTXWE, base + FSPI_INTR); }
> 
> > +}
> > +
> > +static void nxp_fspi_read_rxfifo(struct nxp_fspi *f,
> > +			  const struct spi_mem_op *op)
> > +{
> > +	void __iomem *base = f->iobase;
> > +	int i, j;
> > +	int size, tmp_size, wm_size, ret;
> > +	u32 tmp = 0;
> > +	u8 *buf = op->data.buf.in;
> > +	u32 len = op->data.nbytes;
> > +
> > +	/* Default value of water mark level is 8 bytes. */
> > +	wm_size = 8;
> > +
> > +	while (len > 0) {
> > +		size = len / wm_size;
> > +
> > +		for (i = 0; i < size; i++) {
> > +			/* Wait for RXFIFO available */
> > +			ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > +						   FSPI_INTR_IPRXWA, 0,
> > +						   POLL_TOUT, true);
> > +			WARN_ON(ret);
> > +
> > +			j = 0;
> > +			tmp_size = wm_size;
> > +			while (tmp_size > 0) {
> > +				tmp = 0;
> > +				tmp = fspi_readl(f, base + FSPI_RFDR + j * 4);
> > +				memcpy(buf, &tmp, 4);
> > +				tmp_size -= 4;
> > +				j++;
> > +				buf += 4;
> > +			}
> > +			/* move the FIFO pointer */
> > +			fspi_writel(f, FSPI_INTR_IPRXWA, base + FSPI_INTR);
> > +			len -= wm_size;
> > +		}
> > +
> > +		size = len % wm_size;
> > +
> > +		j = 0;
> > +		if (size) {
> > +			/* Wait for RXFIFO available */
> > +			ret = fspi_readl_poll_tout(f, f->iobase + FSPI_INTR,
> > +						   FSPI_INTR_IPRXWA, 0,
> > +						   POLL_TOUT, true);
> > +			WARN_ON(ret);
> > +
> > +			while (len > 0) {
> > +				tmp = 0;
> > +				size = (len < 4) ? len : 4;
> > +				tmp = fspi_readl(f, base + FSPI_RFDR + j * 4);
> > +				memcpy(buf, &tmp, size);
> > +				len -= size;
> > +				j++;
> > +				buf += size;
> > +			}
> > +		}
> > +
> > +		/* invalid the RXFIFO */
> > +		fspi_writel(f, FSPI_IPRXFCR_CLR, base + FSPI_IPRXFCR);
> > +		/* move the FIFO pointer */
> > +		fspi_writel(f, FSPI_INTR_IPRXWA, base + FSPI_INTR);
> > +	}
> 
> Same here. I think this is overly complicated.
> 
Yes, would do in next version.

> > +}
> > +
> > +static int nxp_fspi_do_op(struct nxp_fspi *f, const struct spi_mem_op
> > +*op) {
> > +	void __iomem *base = f->iobase;
> > +	int seqnum = 0;
> > +	int err = 0;
> > +	u32 reg;
> > +
> > +	reg = fspi_readl(f, base + FSPI_IPRXFCR);
> > +	/* invalid RXFIFO first */
> > +	reg &= ~FSPI_IPRXFCR_DMA_EN;
> > +	reg = reg | FSPI_IPRXFCR_CLR;
> > +	fspi_writel(f, reg, base + FSPI_IPRXFCR);
> > +
> > +	init_completion(&f->c);
> > +
> > +	fspi_writel(f, op->addr.val, base + FSPI_IPCR0);
> > +	/*
> > +	 * Always start the sequence at the same index since we update
> > +	 * the LUT at each exec_op() call. And also specify the DATA
> > +	 * length, since it's has not been specified in the LUT.
> > +	 */
> > +	fspi_writel(f, op->data.nbytes |
> > +		 (SEQID_LUT << FSPI_IPCR1_SEQID_SHIFT) |
> > +		 (seqnum << FSPI_IPCR1_SEQNUM_SHIFT),
> > +		 base + FSPI_IPCR1);
> > +
> > +	/* Trigger the LUT now. */
> > +	fspi_writel(f, FSPI_IPCMD_TRG, base + FSPI_IPCMD);
> > +
> > +	/* Wait for the interrupt. */
> > +	if (!wait_for_completion_timeout(&f->c, msecs_to_jiffies(1000)))
> > +		err = -ETIMEDOUT;
> > +
> > +	/* Invoke IP data read, if request is of data read. */
> > +	if (!err && op->data.nbytes && op->data.dir == SPI_MEM_DATA_IN)
> > +		nxp_fspi_read_rxfifo(f, op);
> > +
> > +	return err;
> > +}
> > +
> > +static int nxp_fspi_exec_op(struct spi_mem *mem, const struct
> > +spi_mem_op *op) {
> > +	struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> > +	int err = 0;
> > +
> > +	mutex_lock(&f->lock);
> > +
> > +	/* Wait for controller being ready. */
> > +	err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0,
> > +				   FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
> > +	WARN_ON(err);
> > +
> > +	nxp_fspi_select_mem(f, mem->spi);
> > +
> > +	nxp_fspi_prepare_lut(f, op);
> > +	/*
> > +	 * If we have large chunks of data, we read them through the AHB bus
> > +	 * by accessing the mapped memory. In all other cases we use
> > +	 * IP commands to access the flash.
> > +	 */
> > +	if (op->data.nbytes > (f->devtype_data->rxfifo - 4) &&
> > +	    op->data.dir == SPI_MEM_DATA_IN) {
> > +		nxp_fspi_read_ahb(f, op);
> > +	} else {
> > +		if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> > +			nxp_fspi_fill_txfifo(f, op);
> > +
> > +		err = nxp_fspi_do_op(f, op);
> > +
> > +		/* Invalidate the data in the AHB buffer. */
> > +		if (op->data.nbytes && op->data.dir == SPI_MEM_DATA_OUT)
> > +			nxp_fspi_invalid(f);
> 
> E.g. in case of an erase operation or a NAND load page operation, the
> invalidation is not triggered, but flash/buffer contents have changed.
> So I'm not sure if this is enough...
Ok, would change this and have invalidate for all operations.

> 
> > +	}
> > +
> > +	mutex_unlock(&f->lock);
> > +
> > +	return err;
> > +}
> > +
> > +static int nxp_fspi_adjust_op_size(struct spi_mem *mem, struct
> > +spi_mem_op *op) {
> > +	struct nxp_fspi *f = spi_controller_get_devdata(mem->spi->master);
> > +
> > +	if (op->data.dir == SPI_MEM_DATA_OUT) {
> > +		if (op->data.nbytes > f->devtype_data->txfifo)
> > +			op->data.nbytes = f->devtype_data->txfifo;
> > +	} else {
> > +		if (op->data.nbytes > f->devtype_data->ahb_buf_size)
> > +			op->data.nbytes = f->devtype_data->ahb_buf_size;
> > +		else if (op->data.nbytes > (f->devtype_data->rxfifo - 4))
> > +			op->data.nbytes = ALIGN_DOWN(op->data.nbytes, 8);
> 
> You are using the same alignments as in the QSPI driver. So AHB reads will
> happen in portions of ahb_buf_size, but you dont' stick to this when you map the
> memory. See above.

Reason mentioned above.

--
Regards
Yogesh Gaur

> 
> Regards,
> Frieder
[...]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 08/15] irqchip: Add RDA8810PL interrupt driver
From: Marc Zyngier @ 2018-12-10  9:39 UTC (permalink / raw)
  To: Manivannan Sadhasivam, olof, arnd, robh+dt, tglx, jason,
	daniel.lezcano, gregkh, jslaby
  Cc: devicetree, linus.walleij, linux-kernel, amit.kucheria,
	linux-serial, overseas.sales, afaerber, linux-arm-kernel,
	zhao_steven
In-Reply-To: <20181128135106.9255-9-manivannan.sadhasivam@linaro.org>

On 28/11/2018 13:50, Manivannan Sadhasivam wrote:
> Add interrupt driver for RDA Micro RDA8810PL SoC.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

Given the dependency on the platform Kconfig entry, how do you want this
to be merged?

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 01/19] dt-bindings: clock: tegra124-dfll: Update DFLL binding for PWM regulator
From: Joseph Lo @ 2018-12-10  9:31 UTC (permalink / raw)
  To: Jon Hunter, Thierry Reding, Peter De Schrijver
  Cc: linux-tegra, devicetree, linux-clk, linux-arm-kernel
In-Reply-To: <e61b1da8-78cd-c3d4-9878-c25fd9c89ba3@nvidia.com>

On 12/10/18 4:59 PM, Jon Hunter wrote:
> 
> On 10/12/2018 08:49, Joseph Lo wrote:
>> Hi Jon,
>>
>> Thanks for reviewing this series.
>>
>> On 12/7/18 9:41 PM, Jon Hunter wrote:
>>>
>>> On 04/12/2018 09:25, Joseph Lo wrote:
>>>> From: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>>
>>>> Add new properties to configure the DFLL PWM regulator support. Also
>>>> add an example and make the I2C clock only required when I2C support is
>>>> used.
>>>>
>>>> Cc: devicetree@vger.kernel.org
>>>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>>> ---
>>>>    .../bindings/clock/nvidia,tegra124-dfll.txt   | 73 ++++++++++++++++++-
>>>>    1 file changed, 71 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>> b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>> index dff236f524a7..8c97600d2bad 100644
>>>> --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.txt
>>>> @@ -8,7 +8,6 @@ the fast CPU cluster. It consists of a free-running
>>>> voltage controlled
>>>>    oscillator connected to the CPU voltage rail (VDD_CPU), and a
>>>> closed loop
>>>>    control module that will automatically adjust the VDD_CPU voltage by
>>>>    communicating with an off-chip PMIC either via an I2C bus or via
>>>> PWM signals.
>>>> -Currently only the I2C mode is supported by these bindings.
>>>>      Required properties:
>>>>    - compatible : should be "nvidia,tegra124-dfll"
>>>> @@ -45,10 +44,28 @@ Required properties for the control loop parameters:
>>>>    Optional properties for the control loop parameters:
>>>>    - nvidia,cg-scale: Boolean value, see the field
>>>> DFLL_PARAMS_CG_SCALE in the TRM.
>>>>    +Optional properties for mode selection:
>>>> +- nvidia,pwm-to-pmic: Use PWM to control regulator rather then I2C.
>>>> +
>>>>    Required properties for I2C mode:
>>>>    - nvidia,i2c-fs-rate: I2C transfer rate, if using full speed mode.
>>>>    -Example:
>>>> +Required properties for PWM mode:
>>>> +- nvidia,pwm-period: period of PWM square wave in microseconds.
>>>> +- nvidia,init-uv: Regulator voltage in micro volts when PWM control
>>>> is disabled.
>>>
>>> Maybe consider 'pwm-inactive-voltage-microvolt'.
>> Ah, I think I need to refine the description here. It should be
>> something like below.
>>   - nvidia,pwm-init-microvolt : Regulator voltage in micro volts when PWM
>> control is initialized
>>
>> This is the initial voltage that when we just initialize the DFLL
>> hardware for PWM output. And before we switch the CPU clock from PLLX to
>> DFLL, we will enable DFLL hardware in closed loop mode which will aplly
>> the DVFS table that was calculated from CVB table.
>>
>> The original description maybe make you think that it's the working
>> voltage when it's under open-loop mode. But it's not. Sorry.
>>
>> When we working on open-loop mode which will switch to low voltage range
>> which also follows the DVFS table. Not this one.
> 
> OK, but I am still not sure what this voltage is. I mean that I
> understand it is the initial voltage, but how exactly do we define this
> number? Where does it come from, how is this determined?
IIRC, this is the same output voltage that bootloader configured with 
proper PLLX rate. So before DFLL handling the CPU clock, that is the 
output voltage. We configure the same when DFLL HW is just initialized.

1 volt here is pretty safe when CPU clock switched from bootloader to 
kernel at 6.912MHz@pllx.

> 
>>>
>>>> +- nvidia,align-offset-uv: Regulator voltage in micro volts when PWM
>>>> control is
>>>> +              enabled and PWM output is low.
>>>
>>> Would this be considered the minimum pwm active voltage?
>> This would be used for minimum voltage for LUT table, which is the table
>> that PMIC can output. The real minimum voltage in PWM mode still depends
>> on the CVB table.
>>
>> So maybe change this one to 'nvidia,pwm-offset-uv'.
> 
> So is this the min supported by the PMIC? Maybe the name should reflect
> that because the above name does not reflect this. Furthermore, if this
> is a min then maybe the name should use 'min' as opposed to 'offset'.
> for example, 'nvidia,pwm-pmic-min-microvolts'.
> 
> Does this need to be described in DT, can it not be queried from the PMIC?
> 
Yes, either way needs to go with DT. The very initial patchset for 
DFLL-PWM support, which introduced a redundant pwm-dfll driver. And with 
the driver, we can describe the PWM vdd-cpu regulator in DT with the 
voltage table that the PMIC can output. But NAKed by reviewer.

Okay, 'nvidia,pwm-pmic-min-microvolts' looks fine. Thanks.

[1]: https://patchwork.ozlabs.org/patch/613524/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH RESEND] drm/meson: remove firmware framebuffers
From: Maxime Jourdan @ 2018-12-10  9:28 UTC (permalink / raw)
  To: Neil Armstrong; +Cc: linux-amlogic, dri-devel, linux-kernel, linux-arm-kernel

In case we are using simplefb or another conflicting framebuffer, make
the call to drm_fb_helper_remove_conflicting_framebuffers()

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
---
 drivers/gpu/drm/meson/meson_drv.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d3443125e661..afbb3d707d15 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -159,6 +159,23 @@ static void meson_vpu_init(struct meson_drm *priv)
 	writel_relaxed(0x20000, priv->io_base + _REG(VPU_WRARB_MODE_L2C1));
 }
 
+static void meson_remove_framebuffers(void)
+{
+	struct apertures_struct *ap;
+
+	ap = alloc_apertures(1);
+	if (!ap)
+		return;
+
+	/* The framebuffer can be located anywhere in RAM */
+	ap->ranges[0].base = 0;
+	ap->ranges[0].size = ~0;
+
+	drm_fb_helper_remove_conflicting_framebuffers(ap, "meson-drm-fb",
+						      false);
+	kfree(ap);
+}
+
 static int meson_drv_bind_master(struct device *dev, bool has_components)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -242,6 +259,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 	if (ret)
 		goto free_drm;
 
+	/* Remove early framebuffers (ie. simplefb) */
+	meson_remove_framebuffers();
+
 	drm_mode_config_init(drm);
 	drm->mode_config.max_width = 3840;
 	drm->mode_config.max_height = 2160;
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH] drm/meson: remove firmware framebuffers
From: Maxime Jourdan @ 2018-12-10  9:08 UTC (permalink / raw)
  To: Neil Armstrong; +Cc: linux-amlogic, dri-devel, linux-kernel, linux-arm-kernel
In-Reply-To: <20181210090448.4301-1-mjourdan@baylibre.com>

On Mon, Dec 10, 2018 at 10:05 AM Maxime Jourdan <mjourdan@baylibre.com> wrote:
>
> In case we are using simplefb or another conflicting framebuffer, make
> the call to drm_fb_helper_remove_conflicting_framebuffers()
>
> Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_drv.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>

Woops, some spaces made it in there.. will resend, sorry about that.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/2] dt-bindings: meson: add specific simplefb bindings
From: Maxime Jourdan @ 2018-12-10  9:06 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-amlogic, devicetree, linux-kernel, linux-arm-kernel,
	Neil Armstrong

Similar to simple-framebuffer-sunxi, we support different display pipelines
that the firmware is free to choose from.

This documents the "amlogic,simple-framebuffer" compatible and the
"amlogic,pipeline" extension.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
---
 .../display/simple-framebuffer-meson.txt      | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/simple-framebuffer-meson.txt

diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer-meson.txt b/Documentation/devicetree/bindings/display/simple-framebuffer-meson.txt
new file mode 100644
index 000000000000..122a5c005cd9
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/simple-framebuffer-meson.txt
@@ -0,0 +1,33 @@
+Meson specific Simple Framebuffer bindings
+
+This binding documents meson specific extensions to the simple-framebuffer
+bindings. The meson simplefb u-boot code relies on the devicetree containing
+pre-populated simplefb nodes.
+
+These extensions are intended so that u-boot can select the right node based
+on which pipeline is being used. As such they are solely intended for
+firmware / bootloader use, and the OS should ignore them.
+
+Required properties:
+- compatible: "amlogic,simple-framebuffer"
+- amlogic,pipeline, one of:
+  "vpu-cvbs"
+  "vpu-hdmi"
+
+Example:
+
+chosen {
+	#address-cells = <2>;
+	#size-cells = <2>;
+	ranges;
+
+	framebuffer@0 {
+		compatible = "amlogic,simple-framebuffer",
+			     "simple-framebuffer";
+		amlogic,pipeline = "vpu-hdmi";
+		clocks = <&clkc CLKID_HDMI_PCLK>,
+			 <&clkc CLKID_CLK81>,
+			 <&clkc CLKID_GCLK_VENCI_INT0>;
+		power-domains = <&pwrc_vpu>;
+	};
+};
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 2/2] arm64: dts: meson-gx: add support for simplefb
From: Maxime Jourdan @ 2018-12-10  9:06 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-amlogic, devicetree, linux-kernel, linux-arm-kernel,
	Neil Armstrong
In-Reply-To: <20181210090640.4495-1-mjourdan@baylibre.com>

SimpleFB allows transferring a framebuffer from the firmware/bootloader
to the kernel, while making sure the related clocks and power supplies
stay enabled.

Add nodes for CVBS and HDMI Simple Framebuffers.

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 25 +++++++++++++++++++++
 arch/arm64/boot/dts/amlogic/meson-gxl.dtsi  | 25 +++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 1ade7e486828..4bc7dd5ba141 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -13,6 +13,31 @@
 / {
 	compatible = "amlogic,meson-gxbb";
 
+	chosen {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		framebuffer@0 {
+			compatible = "amlogic,simple-framebuffer",
+				     "simple-framebuffer";
+			amlogic,pipeline = "vpu-cvbs";
+			power-domains = <&pwrc_vpu>;
+			status = "disabled";
+		};
+
+		framebuffer@1 {
+			compatible = "amlogic,simple-framebuffer",
+				     "simple-framebuffer";
+			amlogic,pipeline = "vpu-hdmi";
+			clocks = <&clkc CLKID_HDMI_PCLK>,
+				 <&clkc CLKID_CLK81>,
+				 <&clkc CLKID_GCLK_VENCI_INT0>;
+			power-domains = <&pwrc_vpu>;
+			status = "disabled";
+		};
+	};
+
 	soc {
 		usb0_phy: phy@c0000000 {
 			compatible = "amlogic,meson-gxbb-usb2-phy";
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 8f0bb3c44bd6..f7ef8e0cf96e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -13,6 +13,31 @@
 / {
 	compatible = "amlogic,meson-gxl";
 
+	chosen {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		framebuffer@0 {
+			compatible = "amlogic,simple-framebuffer",
+				     "simple-framebuffer";
+			amlogic,pipeline = "vpu-cvbs";
+			power-domains = <&pwrc_vpu>;
+			status = "disabled";
+		};
+
+		framebuffer@1 {
+			compatible = "amlogic,simple-framebuffer",
+				     "simple-framebuffer";
+			amlogic,pipeline = "vpu-hdmi";
+			clocks = <&clkc CLKID_HDMI_PCLK>,
+				 <&clkc CLKID_CLK81>,
+				 <&clkc CLKID_GCLK_VENCI_INT0>;
+			power-domains = <&pwrc_vpu>;
+			status = "disabled";
+		};
+	};
+
 	soc {
 		usb0: usb@c9000000 {
 			status = "disabled";
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v9 5/8] KVM: arm64: Support PUD hugepage in stage2_is_exec()
From: Christoffer Dall @ 2018-12-10  9:06 UTC (permalink / raw)
  To: Suzuki K Poulose
  Cc: marc.zyngier, Catalin Marinas, Punit Agrawal, will.deacon,
	linux-kernel, Russell King, punitagrawal, kvmarm,
	linux-arm-kernel
In-Reply-To: <5fddc14d-f6b6-bb97-5f5d-8e1e05e5da95@arm.com>

On Wed, Dec 05, 2018 at 05:57:51PM +0000, Suzuki K Poulose wrote:
> 
> 
> On 01/11/2018 13:38, Christoffer Dall wrote:
> >On Wed, Oct 31, 2018 at 05:57:42PM +0000, Punit Agrawal wrote:
> >>In preparation for creating PUD hugepages at stage 2, add support for
> >>detecting execute permissions on PUD page table entries. Faults due to
> >>lack of execute permissions on page table entries is used to perform
> >>i-cache invalidation on first execute.
> >>
> >>Provide trivial implementations of arm32 helpers to allow sharing of
> >>code.
> >>
> >>Signed-off-by: Punit Agrawal <punit.agrawal@arm.com>
> >>Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >>Cc: Christoffer Dall <christoffer.dall@arm.com>
> >>Cc: Marc Zyngier <marc.zyngier@arm.com>
> >>Cc: Russell King <linux@armlinux.org.uk>
> >>Cc: Catalin Marinas <catalin.marinas@arm.com>
> >>Cc: Will Deacon <will.deacon@arm.com>
> >>---
> >>  arch/arm/include/asm/kvm_mmu.h         |  6 +++
> >>  arch/arm64/include/asm/kvm_mmu.h       |  5 +++
> >>  arch/arm64/include/asm/pgtable-hwdef.h |  2 +
> >>  virt/kvm/arm/mmu.c                     | 53 +++++++++++++++++++++++---
> >>  4 files changed, 61 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> >>index 37bf85d39607..839a619873d3 100644
> >>--- a/arch/arm/include/asm/kvm_mmu.h
> >>+++ b/arch/arm/include/asm/kvm_mmu.h
> >>@@ -102,6 +102,12 @@ static inline bool kvm_s2pud_readonly(pud_t *pud)
> >>  	return false;
> >>  }
> >>+static inline bool kvm_s2pud_exec(pud_t *pud)
> >>+{
> >>+	BUG();
> >
> >nit: I think this should be WARN() now :)
> >
> >>+	return false;
> >>+}
> >>+
> >>  static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> >>  {
> >>  	pte_val(pte) |= L_PTE_S2_RDWR;
> >>diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> >>index 8da6d1b2a196..c755b37b3f92 100644
> >>--- a/arch/arm64/include/asm/kvm_mmu.h
> >>+++ b/arch/arm64/include/asm/kvm_mmu.h
> >>@@ -261,6 +261,11 @@ static inline bool kvm_s2pud_readonly(pud_t *pudp)
> >>  	return kvm_s2pte_readonly((pte_t *)pudp);
> >>  }
> >>+static inline bool kvm_s2pud_exec(pud_t *pudp)
> >>+{
> >>+	return !(READ_ONCE(pud_val(*pudp)) & PUD_S2_XN);
> >>+}
> >>+
> >>  #define hyp_pte_table_empty(ptep) kvm_page_empty(ptep)
> >>  #ifdef __PAGETABLE_PMD_FOLDED
> >>diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> >>index 1d7d8da2ef9b..336e24cddc87 100644
> >>--- a/arch/arm64/include/asm/pgtable-hwdef.h
> >>+++ b/arch/arm64/include/asm/pgtable-hwdef.h
> >>@@ -193,6 +193,8 @@
> >>  #define PMD_S2_RDWR		(_AT(pmdval_t, 3) << 6)   /* HAP[2:1] */
> >>  #define PMD_S2_XN		(_AT(pmdval_t, 2) << 53)  /* XN[1:0] */
> >>+#define PUD_S2_XN		(_AT(pudval_t, 2) << 53)  /* XN[1:0] */
> >>+
> >>  /*
> >>   * Memory Attribute override for Stage-2 (MemAttr[3:0])
> >>   */
> >>diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> >>index 1c669c3c1208..8e44dccd1b47 100644
> >>--- a/virt/kvm/arm/mmu.c
> >>+++ b/virt/kvm/arm/mmu.c
> >>@@ -1083,23 +1083,66 @@ static int stage2_set_pmd_huge(struct kvm *kvm, struct kvm_mmu_memory_cache
> >>  	return 0;
> >>  }
> >>-static bool stage2_is_exec(struct kvm *kvm, phys_addr_t addr)
> >>+/*
> >>+ * stage2_get_leaf_entry - walk the stage2 VM page tables and return
> >>+ * true if a valid and present leaf-entry is found. A pointer to the
> >>+ * leaf-entry is returned in the appropriate level variable - pudpp,
> >>+ * pmdpp, ptepp.
> >>+ */
> >>+static bool stage2_get_leaf_entry(struct kvm *kvm, phys_addr_t addr,
> >>+				  pud_t **pudpp, pmd_t **pmdpp, pte_t **ptepp)
> >
> >Do we need this type madness or could this just return a u64 pointer
> >(NULL if nothing is found) and pass that to kvm_s2pte_exec (because we
> >know it's the same bit we need to check regardless of the pgtable level
> >on both arm and arm64)?
> >
> >Or do we consider that bad for some reason?
> 
> Practically, yes the bit positions are same and thus we should be able
> to do this assuming that it is just a pte. When we get to independent stage2
> pgtable implementation which treats all page table entries as a single type
> with a level information, we should be able to get rid of these.
> But since we have followed the Linux way of page-table manipulation where we
> have "level" specific accessors. The other option is open code the walking
> sequence from the pgd to the leaf entry everywhere.
> 
> I am fine with changing this code, if you like.
> 

Meh, it just looked a bit over-engineered to me when I originally looked
at the patches, but you're right, they align with the rest of the
implementation.

Thanks,

    Christoffer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] drm/meson: remove firmware framebuffers
From: Maxime Jourdan @ 2018-12-10  9:04 UTC (permalink / raw)
  To: Neil Armstrong; +Cc: linux-amlogic, dri-devel, linux-kernel, linux-arm-kernel

In case we are using simplefb or another conflicting framebuffer, make
the call to drm_fb_helper_remove_conflicting_framebuffers()

Signed-off-by: Maxime Jourdan <mjourdan@baylibre.com>
---
 drivers/gpu/drm/meson/meson_drv.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d3443125e661..82fbd7ae80e4 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -159,6 +159,22 @@ static void meson_vpu_init(struct meson_drm *priv)
 	writel_relaxed(0x20000, priv->io_base + _REG(VPU_WRARB_MODE_L2C1));
 }
 
+static void meson_remove_framebuffers(void)
+{
+        struct apertures_struct *ap;
+
+        ap = alloc_apertures(1);
+        if (!ap)
+                return;
+
+        /* The framebuffer can be located anywhere in RAM */
+        ap->ranges[0].base = 0;
+        ap->ranges[0].size = ~0;
+
+        drm_fb_helper_remove_conflicting_framebuffers(ap, "meson-drm-fb", false);
+        kfree(ap);
+}
+
 static int meson_drv_bind_master(struct device *dev, bool has_components)
 {
 	struct platform_device *pdev = to_platform_device(dev);
@@ -242,6 +258,9 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 	if (ret)
 		goto free_drm;
 
+	/* Remove early framebuffers (ie. simplefb) */
+	meson_remove_framebuffers();
+
 	drm_mode_config_init(drm);
 	drm->mode_config.max_width = 3840;
 	drm->mode_config.max_height = 2160;
-- 
2.19.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v10 0/8] Introduce on-chip interconnect API
From: Rafael J. Wysocki @ 2018-12-10  9:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Mark Rutland, Doug Anderson, sanjayc, maxime.ripard,
	Michael Turquette, daidavid1, bjorn.andersson, Saravana Kannan,
	abailon, Lorenzo Pieralisi, Vincent Guittot, seansw, Kevin Hilman,
	evgreen, ksitaraman, devicetree@vger.kernel.org, Arnd Bergmann,
	Linux PM, linux-arm-msm, Rob Herring, linux-tegra, Linux ARM,
	Rafael J. Wysocki, Linux Kernel Mailing List, Amit Kucheria,
	Thierry Reding, Georgi Djakov
In-Reply-To: <20181206145547.GA7884@kroah.com>

On Thu, Dec 6, 2018 at 3:55 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Dec 05, 2018 at 12:41:35PM -0800, Evan Green wrote:
> > On Tue, Nov 27, 2018 at 10:03 AM Georgi Djakov <georgi.djakov@linaro.org> wrote:
> > >
> > > Modern SoCs have multiple processors and various dedicated cores (video, gpu,
> > > graphics, modem). These cores are talking to each other and can generate a
> > > lot of data flowing through the on-chip interconnects. These interconnect
> > > buses could form different topologies such as crossbar, point to point buses,
> > > hierarchical buses or use the network-on-chip concept.
> > >
> > > These buses have been sized usually to handle use cases with high data
> > > throughput but it is not necessary all the time and consume a lot of power.
> > > Furthermore, the priority between masters can vary depending on the running
> > > use case like video playback or CPU intensive tasks.
> > >
> > > Having an API to control the requirement of the system in terms of bandwidth
> > > and QoS, so we can adapt the interconnect configuration to match those by
> > > scaling the frequencies, setting link priority and tuning QoS parameters.
> > > This configuration can be a static, one-time operation done at boot for some
> > > platforms or a dynamic set of operations that happen at run-time.
> > >
> > > This patchset introduce a new API to get the requirement and configure the
> > > interconnect buses across the entire chipset to fit with the current demand.
> > > The API is NOT for changing the performance of the endpoint devices, but only
> > > the interconnect path in between them.
> >
> > For what it's worth, we are ready to land this in Chrome OS. I think
> > this series has been very well discussed and reviewed, hasn't changed
> > much in the last few spins, and is in good enough shape to use as a
> > base for future patches. Georgi's also done a great job reaching out
> > to other SoC vendors, and there appears to be enough consensus that
> > this framework will be usable by more than just Qualcomm. There are
> > also several drivers out on the list trying to add patches to use this
> > framework, with more to come, so it made sense (to us) to get this
> > base framework nailed down. In my experiments this is an important
> > piece of the overall power management story, especially on systems
> > that are mostly idle.
> >
> > I'll continue to track changes to this series and we will ultimately
> > reconcile with whatever happens upstream, but I thought it was worth
> > sending this note to express our "thumbs up" towards this framework.
>
> Looks like a v11 will be forthcoming, so I'll wait for that one to apply
> it to the tree if all looks good.

I'm honestly not sure if it is ready yet.

New versions are coming on and on, which may make such an impression,
but we had some discussion on it at the LPC and some serious questions
were asked during it, for instance regarding the DT binding introduced
here.  I'm not sure how this particular issue has been addressed here,
for example.

Thanks,
Rafael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v9 3/8] KVM: arm/arm64: Introduce helpers to manipulate page table entries
From: Christoffer Dall @ 2018-12-10  9:01 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: suzuki.poulose, marc.zyngier, Catalin Marinas, Punit Agrawal,
	will.deacon, linux-kernel, Russell King, punitagrawal, kvmarm,
	linux-arm-kernel
In-Reply-To: <396402d5-fddf-ab72-01c4-80dd1a0d4d44@arm.com>

On Mon, Dec 03, 2018 at 07:20:08PM +0530, Anshuman Khandual wrote:
> 
> 
> On 10/31/2018 11:27 PM, Punit Agrawal wrote:
> > Introduce helpers to abstract architectural handling of the conversion
> > of pfn to page table entries and marking a PMD page table entry as a
> > block entry.
> 
> Why is this necessary ? we would still need to access PMD, PUD as is
> without any conversion. IOW KVM knows the breakdown of the page table
> at various levels. Is this something required from generic KVM code ?
>   
> > 
> > The helpers are introduced in preparation for supporting PUD hugepages
> > at stage 2 - which are supported on arm64 but do not exist on arm.
> 
> Some of these patches (including the earlier two) are good on it's
> own. Do we have still mention in commit message about the incoming PUD
> enablement as the reason for these cleanup patches ?
> 

Does it hurt?  What is your concern here?


Thanks,

    Christoffer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [linux-sunxi] [PATCH v2 15/15] arm64: dts: allwinner: a64: Add Video Engine node
From: Paul Kocialkowski @ 2018-12-10  9:00 UTC (permalink / raw)
  To: Jernej Škrabec, linux-sunxi
  Cc: devel, devicetree, Sakari Ailus, Maxime Ripard, linux-kernel,
	Hans Verkuil, Chen-Yu Tsai, Rob Herring, Thomas Petazzoni,
	Mark Rutland, Mauro Carvalho Chehab, linux-arm-kernel,
	linux-media
In-Reply-To: <2823800.C4zEU5jiS7@jernej-laptop>

Hi,

On Fri, 2018-12-07 at 22:22 +0100, Jernej Škrabec wrote:
> Hi!
> 
> Dne sreda, 05. december 2018 ob 10:24:44 CET je Paul Kocialkowski napisal(a):
> > This adds the Video Engine node for the A64. Since it can map the whole
> > DRAM range, there is no particular need for a reserved memory node
> > (unlike platforms preceding the A33).
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi index
> > 8557d52c7c99..8d024c10d7cb 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > @@ -397,6 +397,17 @@
> >  			};
> >  		};
> > 
> > +		video-codec@1c0e000 {
> > +			compatible = "allwinner,sun50i-h5-video-engine";
> 
> You meant A64 instead of H5, right?

Ah yes definitely, that's a mistake right here.

I'll send a follow-up patch for switching the compatible to describe
the a64 instead of the h5. In practice, having the a64 use the h5
compatible doesn't cause any issue, but it should be fixed
nevertheless.

Cheers,

Paul

> Best regards,
> Jernej
> 
> > +			reg = <0x01c0e000 0x1000>;
> > +			clocks = <&ccu CLK_BUS_VE>, <&ccu CLK_VE>,
> > +				 <&ccu CLK_DRAM_VE>;
> > +			clock-names = "ahb", "mod", "ram";
> > +			resets = <&ccu RST_BUS_VE>;
> > +			interrupts = <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
> > +			allwinner,sram = <&ve_sram 1>;
> > +		};
> > +
> >  		mmc0: mmc@1c0f000 {
> >  			compatible = "allwinner,sun50i-a64-mmc";
> >  			reg = <0x01c0f000 0x1000>;
> 
> 
> 
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


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