Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH for-5.8 2/2] usb: dwc3: meson-g12a: fix USB2 PHY initialization on G12A and A1 SoCs
From: Greg KH @ 2020-05-27  8:53 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: balbi, hanjie.lin, kernelci.org bot, Martin Blumenstingl,
	linux-usb, linux-kernel, yue.wang, linux-amlogic,
	linux-arm-kernel
In-Reply-To: <40a874eb-1a2b-533e-ee3e-bd90510abaf9@baylibre.com>

On Wed, May 27, 2020 at 10:17:31AM +0200, Neil Armstrong wrote:
> Hi Martin,
> 
> On 26/05/2020 22:29, Martin Blumenstingl wrote:
> > dwc3_meson_g12a_usb2_init_phy() crashes with NULL pointer on an SM1
> > board (which uses the same USB setup as G12A) dereference as reported
> > by the Kernel CI bot. This is because of the following call flow:
> >   dwc3_meson_g12a_probe
> >     priv->drvdata->setup_regmaps
> >       dwc3_meson_g12a_setup_regmaps
> >         priv->usb2_ports is still 0 so priv->u2p_regmap[i] will be NULL
> >     dwc3_meson_g12a_get_phys
> >       initializes priv->usb2_ports
> >     priv->drvdata->usb_init
> >       dwc3_meson_g12a_usb_init
> >         dwc3_meson_g12a_usb_init_glue
> >           dwc3_meson_g12a_usb2_init
> >             priv->drvdata->usb2_init_phy
> >               dwc3_meson_g12a_usb2_init_phy
> >                 dereferences priv->u2p_regmap[i]
> > 
> > Call priv->drvdata->setup_regmaps only after dwc3_meson_g12a_get_phys so
> > priv->usb2_ports is initialized and the regmaps will be set up
> > correctly. This fixes the NULL dereference later on.
> > 
> > Fixes: 013af227f58a97 ("usb: dwc3: meson-g12a: handle the phy and glue registers separately")
> > Reported-by: "kernelci.org bot" <bot@kernelci.org>
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > ---
> >  drivers/usb/dwc3/dwc3-meson-g12a.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
> > index ce5388338389..1f7f4d88ed9d 100644
> > --- a/drivers/usb/dwc3/dwc3-meson-g12a.c
> > +++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
> 
> [...]
> 
> Fixes regression reported at [1] on SEI510 board based on Amlogic G12A.
> 
> Felipe, Greg, can this be queued on uxb-next for 5.8 ?
> 
> Acked-by: Neil Armstrong <narmstron@baylibre.com>

I can take this and patch 1/2 here if Felipe acks them.

thanks,

greg k-h

_______________________________________________
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 08/26] KVM: arm64: Use TTL hint in when invalidating stage-2 translations
From: Marc Zyngier @ 2020-05-27  8:59 UTC (permalink / raw)
  To: Andrew Scull
  Cc: kvm, Andre Przywara, Dave Martin, George Cherian, James Morse,
	Zengtao (B), Catalin Marinas, Will Deacon, kvmarm,
	linux-arm-kernel
In-Reply-To: <20200513090648.GA193035@google.com>

On 2020-05-13 10:06, Andrew Scull wrote:
> On Tue, May 12, 2020 at 01:04:31PM +0100, James Morse wrote:
>> Hi Andrew,
>> 
>> On 07/05/2020 16:13, Andrew Scull wrote:
>> >> @@ -176,7 +177,7 @@ static void clear_stage2_pud_entry(struct kvm_s2_mmu *mmu, pud_t *pud, phys_addr
>> >>  	pmd_t *pmd_table __maybe_unused = stage2_pmd_offset(kvm, pud, 0);
>> >>  	VM_BUG_ON(stage2_pud_huge(kvm, *pud));
>> >>  	stage2_pud_clear(kvm, pud);
>> >> -	kvm_tlb_flush_vmid_ipa(mmu, addr);
>> >> +	kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT);
>> >>  	stage2_pmd_free(kvm, pmd_table);
>> >>  	put_page(virt_to_page(pud));
>> >>  }
>> >> @@ -186,7 +187,7 @@ static void clear_stage2_pmd_entry(struct kvm_s2_mmu *mmu, pmd_t *pmd, phys_addr
>> >>  	pte_t *pte_table = pte_offset_kernel(pmd, 0);
>> >>  	VM_BUG_ON(pmd_thp_or_huge(*pmd));
>> >>  	pmd_clear(pmd);
>> >> -	kvm_tlb_flush_vmid_ipa(mmu, addr);
>> >> +	kvm_tlb_flush_vmid_ipa(mmu, addr, S2_NO_LEVEL_HINT);
>> >>  	free_page((unsigned long)pte_table);
>> >>  	put_page(virt_to_page(pmd));
>> >>  }
>> >
>> > Going by the names, is it possible to give a better level hint for these
>> > cases?
>> 
>> There is no leaf entry being invalidated here. After clearing the 
>> range, we found we'd
>> emptied (and invalidated) a whole page of mappings:
>> |	if (stage2_pmd_table_empty(kvm, start_pmd))
>> |		clear_stage2_pud_entry(mmu, pud, start_addr);
>> 
>> Now we want to remove the link to the empty page so we can free it. We 
>> are changing the
>> structure of the tables, not what gets mapped.
>> 
>> I think this is why we need the un-hinted behaviour, to invalidate 
>> "any level of the
>> translation table walk required to translate the specified IPA". 
>> Otherwise the hardware
>> can look for a leaf at the indicated level, find none, and do nothing.
>> 
>> 
>> This is sufficiently horrible, its possible I've got it completely 
>> wrong! (does it make
>> sense?)
> 
> Ok. `addr` is an IPA, that IPA is now omitted from the map so doesn't
> appear in any entry of the table, least of all a leaf entry. That makes
> sense.
> 
> Is there a convention to distinguish IPA and PA similar to the
> distinction for VA or does kvmarm just use phys_addr_t all round?
> 
> It seems like the TTL patches are failry self contained if it would be
> easier to serparate them out from these larger series?

They are. This whole series is a mix of unrelated patches anyway.
Their only goal is to make my life a bit easier in the distant
future.

I'll repost that anyway, as I have made some cosmetic changes.

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 0/2] Introduce PCI_FIXUP_IOMMU
From: Greg Kroah-Hartman @ 2020-05-27  9:00 UTC (permalink / raw)
  To: Zhangfei Gao
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
	linux-pci, Joerg Roedel, Hanjun Guo, Rafael J. Wysocki,
	linux-kernel, iommu, linux-acpi, Wangzhou, linux-crypto,
	Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012, linux-arm-kernel,
	Len Brown
In-Reply-To: <1590493749-13823-1-git-send-email-zhangfei.gao@linaro.org>

On Tue, May 26, 2020 at 07:49:07PM +0800, Zhangfei Gao wrote:
> Some platform devices appear as PCI but are actually on the AMBA bus,

Why would these devices not just show up on the AMBA bus and use all of
that logic instead of being a PCI device and having to go through odd
fixes like this?

thanks,

greg k-h

_______________________________________________
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 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Marco Felsch @ 2020-05-27  9:00 UTC (permalink / raw)
  To: Fengping Yu
  Cc: Dmitry Torokhov, linux-mediatek, linux-arm-kernel,
	Andy Shevchenko, linux-input
In-Reply-To: <20200527083137.56566-3-fengping.yu@mediatek.com>

Hi,

we are in a very good state now :)

On 20-05-27 16:31, Fengping Yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
> 
> This adds matrix keypad support for Mediatek SoCs.
> 
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>
> ---
>  drivers/input/keyboard/Kconfig   |  11 ++
>  drivers/input/keyboard/Makefile  |   1 +
>  drivers/input/keyboard/mtk-kpd.c | 219 +++++++++++++++++++++++++++++++
>  3 files changed, 231 insertions(+)
>  create mode 100644 drivers/input/keyboard/mtk-kpd.c
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 28de965a08d5..f4f58320d14f 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -782,6 +782,17 @@ config KEYBOARD_BCM
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called bcm-keypad.
>  
> +config KEYBOARD_MTK_KPD
> +	tristate "MediaTek Keypad Support"
> +	depends on (ARCH_MEDIATEK && ARM) || COMPILE_TEST

So the driver is only for 32bit Mediatek SoC's?

> +	select CONFIG_REGMAP_MMIO
> +	select INPUT_MATRIXKMAP
> +	help
> +	  Say Y here if you want to use the keypad on MediaTek SoCs.
> +	  If unsure, say N.
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called mtk-kpd.
> +
>  config KEYBOARD_MTK_PMIC
>  	tristate "MediaTek PMIC keys support"
>  	depends on MFD_MT6397
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 1d689fdd5c00..6c9d852c377e 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
>  obj-$(CONFIG_KEYBOARD_MAX7359)		+= max7359_keypad.o
>  obj-$(CONFIG_KEYBOARD_MCS)		+= mcs_touchkey.o
>  obj-$(CONFIG_KEYBOARD_MPR121)		+= mpr121_touchkey.o
> +obj-$(CONFIG_KEYBOARD_MTK_KPD)		+= mtk-kpd.o
>  obj-$(CONFIG_KEYBOARD_MTK_PMIC) 	+= mtk-pmic-keys.o
>  obj-$(CONFIG_KEYBOARD_NEWTON)		+= newtonkbd.o
>  obj-$(CONFIG_KEYBOARD_NOMADIK)		+= nomadik-ske-keypad.o
> diff --git a/drivers/input/keyboard/mtk-kpd.c b/drivers/input/keyboard/mtk-kpd.c
> new file mode 100644
> index 000000000000..6e18aad1f476
> --- /dev/null
> +++ b/drivers/input/keyboard/mtk-kpd.c
> @@ -0,0 +1,219 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 MediaTek Inc.
> + * Author Terry Chang <terry.chang@mediatek.com>
> + */
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MTK_KPD_NAME		"mtk-kpd"
> +#define MTK_KPD_MEM		0x0004
> +#define MTK_KPD_DEBOUNCE	0x0018
> +#define MTK_KPD_DEBOUNCE_MASK	GENMASK(13, 0)
> +#define MTK_KPD_DEBOUNCE_MAX_US	256000
> +#define MTK_KPD_NUM_MEMS	5
> +#define MTK_KPD_NUM_BITS	136	/* 4*32+8 MEM5 only use 8 BITS */
> +
> +struct mtk_keypad {
> +	struct regmap *regmap;
> +	struct input_dev *input_dev;
> +	struct clk *clk;
> +	void __iomem *base;
> +	bool wakeup;

Nitpick:
The wakeup member is only used during probe() so this can probably
removed from the driver struct. This should only be addressed if you
need to send a v10.

> +	u32 n_rows;
> +	u32 n_cols;
> +	DECLARE_BITMAP(keymap_state, MTK_KPD_NUM_BITS);
> +};
> +
> +static const struct regmap_config keypad_regmap_cfg = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = sizeof(u32),
> +	.max_register = 36,
> +};
> +
> +static irqreturn_t kpd_irq_handler(int irq, void *dev_id)
> +{
> +	struct mtk_keypad *keypad = dev_id;
> +	unsigned short *keycode = keypad->input_dev->keycode;
> +	DECLARE_BITMAP(new_state, MTK_KPD_NUM_BITS);
> +	DECLARE_BITMAP(change, MTK_KPD_NUM_BITS);
> +	int bit_nr;
> +	int pressed;
> +	unsigned short code;
> +
> +	regmap_raw_read(keypad->regmap, MTK_KPD_MEM,
> +			new_state, MTK_KPD_NUM_MEMS);
> +
> +	bitmap_xor(change, new_state, keypad->keymap_state, MTK_KPD_NUM_BITS);
> +
> +	for_each_set_bit(bit_nr, change, MTK_KPD_NUM_BITS) {
> +		/* 1: not pressed, 0: pressed */
> +		pressed = !test_bit(bit_nr, new_state);
> +		dev_dbg(&keypad->input_dev->dev, "%s",
> +			pressed ? "pressed" : "released");
> +
> +		/* 32bit register only use low 16bit as keypad mem register */
> +		code = keycode[bit_nr - 16 * (BITS_TO_U32(bit_nr) - 1)];
> +
> +		input_report_key(keypad->input_dev, code, pressed);
> +		input_sync(keypad->input_dev);
> +
> +		dev_dbg(&keypad->input_dev->dev,
> +			"report Linux keycode = %d\n", code);
> +	}
> +
> +	bitmap_copy(keypad->keymap_state, new_state, MTK_KPD_NUM_BITS);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void kpd_clk_disable(void *data)
> +{
> +	clk_disable_unprepare(data);
> +}
> +
> +static int kpd_pdrv_probe(struct platform_device *pdev)
> +{
> +	struct mtk_keypad *keypad;
> +	struct pinctrl *keypad_pinctrl;
> +	struct pinctrl_state *kpd_default;
> +	unsigned int irqnr;
> +	u32 debounce;
> +	int ret;
> +
> +	keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
> +	if (!keypad)
> +		return -ENOMEM;
> +
> +	keypad->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(keypad->base))
> +		return PTR_ERR(keypad->base);
> +
> +	keypad->regmap = devm_regmap_init_mmio(&pdev->dev,
> +						keypad->base,
> +						&keypad_regmap_cfg);
> +	if (IS_ERR(keypad->regmap)) {
> +		dev_err(&pdev->dev,
> +			"regmap init failed:%ld\n", PTR_ERR(keypad->regmap));
> +		return PTR_ERR(keypad->regmap);
> +	}
> +
> +	bitmap_fill(keypad->keymap_state, MTK_KPD_NUM_BITS);
> +
> +	keypad->input_dev = devm_input_allocate_device(&pdev->dev);
> +	if (!keypad->input_dev) {
> +		dev_err(&pdev->dev, "Failed to allocate input dev\n");
> +		return -ENOMEM;
> +	}
> +
> +	keypad->input_dev->name = MTK_KPD_NAME;
> +	keypad->input_dev->id.bustype = BUS_HOST;
> +
> +	ret = matrix_keypad_parse_properties(&pdev->dev, &keypad->n_rows,
> +					     &keypad->n_cols);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to parse keypad params\n");
> +		return ret;
> +	}
> +
> +	if (device_property_read_u32(&pdev->dev, "mediatek,debounce-us",
> +				     &debounce))
> +		debounce = 16000;
> +
> +	if (debounce > MTK_KPD_DEBOUNCE_MAX_US) {
> +		dev_err(&pdev->dev, "Debounce time exceeds the maximum allowed time %dus\n",
> +			MTK_KPD_DEBOUNCE_MAX_US);
> +		return -EINVAL;
> +	}
> +
> +	keypad->wakeup = device_property_read_bool(&pdev->dev, "wakeup-source");
> +
> +	dev_dbg(&pdev->dev, "n_row=%d n_col=%d debounce=%d\n",
> +		keypad->n_rows, keypad->n_cols, debounce);
> +
> +	ret = matrix_keypad_build_keymap(NULL, NULL,
> +					keypad->n_rows,
> +					keypad->n_cols,
> +					NULL,
> +					keypad->input_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to build keymap\n");
> +		return ret;
> +	}
> +
> +	regmap_write(keypad->regmap, KP_DEBOUNCE,
> +		     debounce * 32 / 1000 & MTK_KPD_DEBOUNCE_MASK);
> +
> +	keypad->clk = devm_clk_get(&pdev->dev, "kpd");
> +	if (IS_ERR(clk))
> +		return clk;
> +
> +	ret = clk_prepare_enable(keypad->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "cannot prepare/enable keypad clock\n");
> +		return ret;
> +	}
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, kpd_clk_disable,
> +					keypad->clk);
> +	if (ret)
> +		return ret;
> +
> +	keypad_pinctrl = devm_pinctrl_get(&pdev->dev);
> +	if (IS_ERR(keypad_pinctrl))
> +		return PTR_ERR(keypad_pinctrl);
> +
> +	kpd_default = pinctrl_lookup_state(keypad_pinctrl, "default");
> +	if (IS_ERR(kpd_default))
> +		return PTR_ERR(kpd_default);
> +
> +	pinctrl_select_state(keypad_pinctrl, kpd_default);

You only request the "default" state here. This is handled by the driver
core (pinctrl_bind_pins()) and can be dropped here and we can also
remove the "linux/pinctrl/consumer.h" include. Sorry for this late finding.

> +
> +	irqnr = platform_get_irq(pdev, 0);
> +	if (irqnr < 0)
> +		return -irqnr;
		       ^
		       irqnr is already negative

> +	ret = devm_request_threaded_irq(&pdev->dev, irqnr,
> +					NULL, kpd_irq_handler, 0,
> +					MTK_KPD_NAME, keypad);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
> +						irqnr, ret);

Still not alligned.

> +		return ret;
> +	}
> +
> +	ret = input_register_device(keypad->input_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to register device\n");
> +		return ret;
> +	}
> +
> +	return device_init_wakeup(&pdev->dev, keypad->wakeup);
> +}
> +
> +static const struct of_device_id kpd_of_match[] = {
> +	{.compatible = "mediatek, mt6779-keypad"},
> +	{.compatible = "mediatek,kp"},
> +	{/* sentinel */}

There are still missing whitespaces and checkpatch should complain
about all three entries, again please use it! 

Regards,
  Marco

> +};
> +
> +static struct platform_driver kpd_pdrv = {
> +	.probe = kpd_pdrv_probe,
> +	.driver = {
> +		   .name = MTK_KPD_NAME,
> +		   .of_match_table = kpd_of_match,
> +	},
> +};
> +module_platform_driver(kpd_pdrv);
> +
> +MODULE_AUTHOR("Mediatek Corporation");
> +MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.18.0

_______________________________________________
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 2/2] iommu: calling pci_fixup_iommu in iommu_fwspec_init
From: Greg Kroah-Hartman @ 2020-05-27  9:01 UTC (permalink / raw)
  To: Zhangfei Gao
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
	linux-pci, Joerg Roedel, Hanjun Guo, Rafael J. Wysocki,
	linux-kernel, iommu, linux-acpi, Wangzhou, linux-crypto,
	Sudeep Holla, Bjorn Helgaas, kenneth-lee-2012, linux-arm-kernel,
	Len Brown
In-Reply-To: <1590493749-13823-3-git-send-email-zhangfei.gao@linaro.org>

On Tue, May 26, 2020 at 07:49:09PM +0800, Zhangfei Gao wrote:
> Calling pci_fixup_iommu in iommu_fwspec_init, which alloc
> iommu_fwnode. Some platform devices appear as PCI but are
> actually on the AMBA bus, and they need fixup in
> drivers/pci/quirks.c handling iommu_fwnode.
> So calling pci_fixup_iommu after iommu_fwnode is allocated.
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> ---
>  drivers/iommu/iommu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 7b37542..fb84c42 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2418,6 +2418,10 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode,
>  	fwspec->iommu_fwnode = iommu_fwnode;
>  	fwspec->ops = ops;
>  	dev_iommu_fwspec_set(dev, fwspec);
> +
> +	if (dev_is_pci(dev))
> +		pci_fixup_device(pci_fixup_iommu, to_pci_dev(dev));

Why can't the caller do this as it "knows" it is a PCI device at that
point in time, right?

thanks,

greg k-h

_______________________________________________
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 1/2] PCI: Introduce PCI_FIXUP_IOMMU
From: Greg Kroah-Hartman @ 2020-05-27  9:01 UTC (permalink / raw)
  To: Zhangfei Gao
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, Arnd Bergmann,
	linux-pci, Joerg Roedel, Sudeep Holla, Rafael J. Wysocki,
	linux-kernel, iommu, Christoph Hellwig, linux-acpi, Wangzhou,
	linux-crypto, Hanjun Guo, Bjorn Helgaas, kenneth-lee-2012,
	linux-arm-kernel, Len Brown
In-Reply-To: <39144dc0-3b04-3127-978b-bd8487dd06e0@linaro.org>

On Tue, May 26, 2020 at 11:09:57PM +0800, Zhangfei Gao wrote:
> Hi, Christoph
> 
> On 2020/5/26 下午10:46, Christoph Hellwig wrote:
> > On Tue, May 26, 2020 at 07:49:08PM +0800, Zhangfei Gao wrote:
> > > Some platform devices appear as PCI but are actually on the AMBA bus,
> > > and they need fixup in drivers/pci/quirks.c handling iommu_fwnode.
> > > Here introducing PCI_FIXUP_IOMMU, which is called after iommu_fwnode
> > > is allocated, instead of reusing PCI_FIXUP_FINAL since it will slow
> > > down iommu probing as all devices in fixup final list will be
> > > reprocessed.
> > Who is going to use this?  I don't see a single user in the series.
> We will add iommu fixup in drivers/pci/quirks.c, handling
> 
> fwspec->can_stall, which is introduced in
> 
> https://www.spinics.net/lists/linux-pci/msg94559.html
> 
> Unfortunately, the patch does not catch v5.8, so we have to wait.
> And we want to check whether this is a right method to solve this issue.

We can't take new apis without a real user, so please submit them all at
once.

thanks,

greg k-h

_______________________________________________
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/3] configs: defconfig: Add CONFIG_KEYBOARD_MTK_KPD=m
From: Marco Felsch @ 2020-05-27  9:02 UTC (permalink / raw)
  To: Fengping Yu
  Cc: Dmitry Torokhov, linux-mediatek, linux-arm-kernel,
	Andy Shevchenko, linux-input
In-Reply-To: <20200527083137.56566-4-fengping.yu@mediatek.com>

On 20-05-27 16:31, Fengping Yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>
> 
> Add Mediatek matrix keypad support in defconfig.
> 
> Signed-off-by: fengping.yu <fengping.yu@mediatek.com>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

> ---
>  arch/arm64/configs/defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 24e534d85045..112ced090b21 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -349,6 +349,7 @@ CONFIG_KEYBOARD_GPIO=y
>  CONFIG_KEYBOARD_SNVS_PWRKEY=m
>  CONFIG_KEYBOARD_IMX_SC_KEY=m
>  CONFIG_KEYBOARD_CROS_EC=y
> +CONFIG_KEYBOARD_MTK_KPD=m
>  CONFIG_INPUT_TOUCHSCREEN=y
>  CONFIG_TOUCHSCREEN_ATMEL_MXT=m
>  CONFIG_INPUT_MISC=y
> -- 
> 2.18.0

_______________________________________________
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 0/8] soc: Use custom soc attribute group and DEVICE_ATTR_RO
From: Greg Kroah-Hartman @ 2020-05-27  9:03 UTC (permalink / raw)
  To: Sudeep Holla; +Cc: Arnd Bergmann, linux-kernel, linux-arm-kernel
In-Reply-To: <20200523170859.50003-1-sudeep.holla@arm.com>

On Sat, May 23, 2020 at 06:08:51PM +0100, Sudeep Holla wrote:
> Hi,
> 
> While trying to add SMCCC based SOC_ID support, I found the custom soc
> attribute group which had no users in the tree and check if it can be
> used or can be removed. I realised that it could clean up the custom
> attributes that are added using device_create_file today.

Whole series looks good, nice job.

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

^ permalink raw reply

* Re: [RFC 0/2] iommu: arm-smmu: Add support for early direct mappings
From: Laurentiu Tudor @ 2020-05-27  9:06 UTC (permalink / raw)
  To: John Stultz, Bjorn Andersson
  Cc: Will Deacon, linux-arm-msm, iommu, Thierry Reding, linux-tegra,
	Robin Murphy, linux-arm-kernel
In-Reply-To: <CALAqxLVmomdKJCwh=e-PX+8-seDX0RXA81FzmG4sEyJmbXBh9A@mail.gmail.com>


On 5/26/2020 11:34 PM, John Stultz wrote:
> On Thu, May 14, 2020 at 12:34 PM <bjorn.andersson@linaro.org> wrote:
>>
>> On Thu 27 Feb 18:57 PST 2020, Bjorn Andersson wrote:
>>
>> Rob, Will, we're reaching the point where upstream has enough
>> functionality that this is becoming a critical issue for us.
>>
>> E.g. Lenovo Yoga C630 is lacking this and a single dts patch to boot
>> mainline with display, GPU, WiFi and audio working and the story is
>> similar on several devboards.
>>
>> As previously described, the only thing I want is the stream mapping
>> related to the display controller in place, either with the CB with
>> translation disabled or possibly with a way to specify the framebuffer
>> region (although this turns out to mess things up in the display
>> driver...)
>>
>> I did pick this up again recently and concluded that by omitting the
>> streams for the USB controllers causes an instability issue seen on one
>> of the controller to disappear. So I would prefer if we somehow could
>> have a mechanism to only pick the display streams and the context
>> allocation for this.
>>
>>
>> Can you please share some pointers/insights/wishes for how we can
>> conclude on this subject?
> 
> Ping? I just wanted to follow up on this discussion as this small
> series is crucial for booting mainline on the Dragonboard 845c
> devboard. It would be really valuable to be able to get some solution
> upstream so we can test mainline w/o adding additional patches.

+1

There are also some NXP chips that depend on this. Also, I've submitted
a v2 [1] a while back that tries to address the feedback on the initial
implementation.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=164853

---
Best Regards, Laurentiu

_______________________________________________
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 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Marco Felsch @ 2020-05-27  9:09 UTC (permalink / raw)
  To: Fengping Yu
  Cc: Dmitry Torokhov, linux-mediatek, linux-arm-kernel,
	Andy Shevchenko, linux-input
In-Reply-To: <20200527083137.56566-3-fengping.yu@mediatek.com>

Hi,

just found another typo.

On 20-05-27 16:31, Fengping Yu wrote:
> From: "fengping.yu" <fengping.yu@mediatek.com>

...

> +static const struct of_device_id kpd_of_match[] = {
> +	{.compatible = "mediatek, mt6779-keypad"},
				 ^
			This is wrong

> +	{.compatible = "mediatek,kp"},

I would also drop the generic "mediatek,kp" compatible. Instead we
should list all supported SoCs here.

Regards,
  Marco

> +	{/* sentinel */}
> +};
> +
> +static struct platform_driver kpd_pdrv = {
> +	.probe = kpd_pdrv_probe,
> +	.driver = {
> +		   .name = MTK_KPD_NAME,
> +		   .of_match_table = kpd_of_match,
> +	},
> +};
> +module_platform_driver(kpd_pdrv);
> +
> +MODULE_AUTHOR("Mediatek Corporation");
> +MODULE_DESCRIPTION("MTK Keypad (KPD) Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.18.0

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

^ permalink raw reply

* Re: [V6, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-05-27  9:01 UTC (permalink / raw)
  To: Tomasz Figa
  Cc: Mark Rutland, Nicolas Boichat, Andy Shevchenko, srv_heupstream,
	linux-devicetree, Linus Walleij,
	Shengnan Wang (王圣男), Louis Kuo,
	Bartosz Golaszewski, Sj Huang, Rob Herring,
	moderated list:ARM/Mediatek SoC support, dongchun.zhu,
	Sakari Ailus, Matthias Brugger, Cao Bing Bu,
	Mauro Carvalho Chehab,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg  Roedel <joro@8bytes.org>, ,
	Linux Media Mailing List
In-Reply-To: <CAAFQd5CCsT_oM9aij_imV+NABzByi94RmCj97Dx0Tk3S0WDsTg@mail.gmail.com>

Hi Tomasz,

On Mon, 2020-05-25 at 13:45 +0200, Tomasz Figa wrote:
> On Fri, May 22, 2020 at 11:27 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > Thanks for the review. My replies are as below.
> >
> > On Thu, 2020-05-21 at 19:51 +0000, Tomasz Figa wrote:
> > > Hi Dongchun, Sakari,
> > >
> > > On Mon, May 18, 2020 at 09:27:31PM +0800, Dongchun Zhu wrote:
> [snip]
> > > > +   pm_runtime_enable(dev);
> > > > +   if (!pm_runtime_enabled(dev)) {
> > > > +           ret = dw9768_runtime_resume(dev);
> > > > +           if (ret < 0) {
> > > > +                   dev_err(dev, "failed to power on: %d\n", ret);
> > > > +                   goto entity_cleanup;
> > > > +           }
> > > > +   }
> > > > +
> > > > +   ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > +   if (ret < 0)
> > > > +           goto entity_cleanup;
> > > > +
> > > > +   return 0;
> > > > +
> > > > +entity_cleanup:
> > >
> > > Need to power off if the code above powered on.
> > >
> >
> > Thanks for the reminder.
> > If there is something wrong with runtime PM, actuator is to be powered
> > on via dw9768_runtime_resume() API.
> > When actuator sub-device is powered on completely and async registered
> > successfully, we shall power off it afterwards.
> >
> 
> The code above calls dw9768_runtime_resume() if
> !pm_runtime_enabled(dev), but the clean-up code below the
> entity_cleanup label doesn't have the corresponding
> dw9768_runtime_suspend() call.
> 

Did you mean the 'entity_cleanup' after v4l2_async_register_subdev()?
Actually I made some changes for OV02A V9, according to this comment.
Could you help review that change? Thanks.

> Best regards,
> Tomasz

_______________________________________________
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 00/91] drm/vc4: Support BCM2711 Display Pipelin
From: Maxime Ripard @ 2020-05-27  9:13 UTC (permalink / raw)
  To: Daniel Drake
  Cc: linux-arm-kernel, devicetree, Linux Kernel, dri-devel,
	Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel,
	Jian-Hong Pan, Linux Upstreaming Team, linux-clk,
	Nicolas Saenz Julienne, linux-i2c
In-Reply-To: <CAD8Lp467DiYWLwH6T1Jeq-uyN4VEuef-gGWw0_bBTtmSPr00Ag@mail.gmail.com>


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

Hi Daniel,

On Wed, May 27, 2020 at 11:49:36AM +0800, Daniel Drake wrote:
> Hi Maxime,
> 
> On Tue, May 26, 2020 at 6:20 PM Maxime Ripard <maxime@cerno.tech> wrote:
> > I gave it a try with U-Boot with my latest work and couldn't reproduce it, so it
> > seems that I fixed it along the way
> 
> Is your latest work available in a git branch anywhere that we could
> test directly?

I'm about to send a v3 today or tomorrow, I can Cc you (and Jian-Hong) if you
want.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 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: [RFC] Use SMMU HTTU for DMA dirty page tracking
From: Jean-Philippe Brucker @ 2020-05-27  9:14 UTC (permalink / raw)
  To: Tian, Kevin
  Cc: Zhao, Yan Y, Suzuki K Poulose, maz@kernel.org,
	alex.williamson@redhat.com, iommu@lists.linux-foundation.org,
	Xiang Zheng, James Morse, linux-arm-kernel@lists.infradead.org,
	prime.zeng@hisilicon.com, Kirti Wankhede, Wang Haibin,
	Will Deacon, kvmarm@lists.cs.columbia.edu,
	julien.thierry.kdev@gmail.com
In-Reply-To: <MWHPR11MB1645AA007D24F2D005794E5C8CB10@MWHPR11MB1645.namprd11.prod.outlook.com>

On Wed, May 27, 2020 at 08:40:47AM +0000, Tian, Kevin wrote:
> > From: Xiang Zheng <zhengxiang9@huawei.com>
> > Sent: Wednesday, May 27, 2020 2:45 PM
> > 
> > 
> > On 2020/5/27 11:27, Tian, Kevin wrote:
> > >> From: Xiang Zheng
> > >> Sent: Monday, May 25, 2020 7:34 PM
> > >>
> > >> [+cc Kirti, Yan, Alex]
> > >>
> > >> On 2020/5/23 1:14, Jean-Philippe Brucker wrote:
> > >>> Hi,
> > >>>
> > >>> On Tue, May 19, 2020 at 05:42:55PM +0800, Xiang Zheng wrote:
> > >>>> Hi all,
> > >>>>
> > >>>> Is there any plan for enabling SMMU HTTU?
> > >>>
> > >>> Not outside of SVA, as far as I know.
> > >>>
> > >>
> > >>>> I have seen the patch locates in the SVA series patch, which adds
> > >>>> support for HTTU:
> > >>>>     https://www.spinics.net/lists/arm-kernel/msg798694.html
> > >>>>
> > >>>> HTTU reduces the number of access faults on SMMU fault queue
> > >>>> (permission faults also benifit from it).
> > >>>>
> > >>>> Besides reducing the faults, HTTU also helps to track dirty pages for
> > >>>> device DMA. Is it feasible to utilize HTTU to get dirty pages on device
> > >>>> DMA during VFIO live migration?
> > >>>
> > >>> As you know there is a VFIO interface for this under discussion:
> > >>> https://lore.kernel.org/kvm/1589781397-28368-1-git-send-email-
> > >> kwankhede@nvidia.com/
> > >>> It doesn't implement an internal API to communicate with the IOMMU
> > >> driver
> > >>> about dirty pages.
> > >
> > > We plan to add such API later, e.g. to utilize A/D bit in VT-d 2nd-level
> > > page tables (Rev 3.0).
> > >
> > 
> > Thank you, Kevin.
> > 
> > When will you send this series patches? Maybe(Hope) we can also support
> > hardware-based dirty pages tracking via common APIs based on your
> > patches. :)
> 
> Yan is working with Kirti on basic live migration support now. After that
> part is done, we will start working on A/D bit support. Yes, common APIs
> are definitely the goal here.
> 
> > 
> > >>
> > >>>
> > >>>> If SMMU can track dirty pages, devices are not required to implement
> > >>>> additional dirty pages tracking to support VFIO live migration.
> > >>>
> > >>> It seems feasible, though tracking it in the device might be more
> > >>> efficient. I might have misunderstood but I think for live migration of
> > >>> the Intel NIC they trap guest accesses to the device and introspect its
> > >>> state to figure out which pages it is accessing.
> > >
> > > Does HTTU implement A/D-like mechanism in SMMU page tables, or just
> > > report dirty pages in a log buffer? Either way tracking dirty pages in IOMMU
> > > side is generic thus doesn't require device-specific tweak like in Intel NIC.
> > >
> > 
> > Currently HTTU just implement A/D-like mechanism in SMMU page tables.
> > We certainly
> > expect SMMU can also implement PML-like feature so that we can avoid
> > walking the
> > whole page table to get the dirty pages.

There is no reporting of dirty pages in log buffer. It might be possible
to do software logging based on PRI or Stall, but that requires special
support in the endpoint as well as the SMMU.

> Is there a link to HTTU introduction?

I don't know any gentle introduction, but there are sections D5.4.11
"Hardware management of the Access flag and dirty state" in the ARM
Architecture Reference Manual (DDI0487E), and section 3.13 "Translation
table entries and Access/Dirty flags" in the SMMU specification
(IHI0070C). HTTU stands for "Hardware Translation Table Update".

In short, when HTTU is enabled, the SMMU translation performs an atomic
read-modify-write on the leaf translation table descriptor, setting some
bits depending on the type of memory access. This can be enabled
independently on both stage-1 and stage-2 tables (equivalent to your 1st
and 2nd page tables levels, I think).

Thanks,
Jean

_______________________________________________
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 00/91] drm/vc4: Support BCM2711 Display Pipelin
From: Daniel Drake @ 2020-05-27  9:15 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-arm-kernel, devicetree, Linux Kernel, dri-devel,
	Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel,
	Jian-Hong Pan, Linux Upstreaming Team, linux-clk,
	Nicolas Saenz Julienne, linux-i2c
In-Reply-To: <20200527091335.7wc3uy67lbz7j4di@gilmour.lan>

On Wed, May 27, 2020 at 5:13 PM Maxime Ripard <maxime@cerno.tech> wrote:
> I'm about to send a v3 today or tomorrow, I can Cc you (and Jian-Hong) if you
> want.

That would be great, although given the potentially inconsistent
results we've been seeing so far it would be great if you could
additionally push a git branch somewhere.
That way we can have higher confidence that we are applying exactly
the same patches to the same base etc.

Thanks
Daniel

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

^ permalink raw reply

* [PATCH] net: ethernet: mtk-star-emac: fix error path in RX handling
From: Bartosz Golaszewski @ 2020-05-27  9:24 UTC (permalink / raw)
  To: John Crispin, Sean Wang, Mark Lee, David S . Miller,
	Jakub Kicinski, Matthias Brugger
  Cc: Stephane Le Provost, Bartosz Golaszewski, netdev, linux-kernel,
	Fabien Parent, linux-mediatek, Andrew Perepech, Pedro Tsai,
	Nathan Chancellor, linux-arm-kernel

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The dma_addr field in desc_data must not be overwritten until after the
new skb is mapped. Currently we do replace it with uninitialized value
in error path. This change fixes it by moving the assignment before the
label to which we jump after mapping or allocation errors.

Fixes: 8c7bd5a454ff ("net: ethernet: mtk-star-emac: new driver")
Reported-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/net/ethernet/mediatek/mtk_star_emac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_star_emac.c b/drivers/net/ethernet/mediatek/mtk_star_emac.c
index b74349cede28..72bb624a6a68 100644
--- a/drivers/net/ethernet/mediatek/mtk_star_emac.c
+++ b/drivers/net/ethernet/mediatek/mtk_star_emac.c
@@ -1308,6 +1308,8 @@ static int mtk_star_receive_packet(struct mtk_star_priv *priv)
 		goto push_new_skb;
 	}
 
+	desc_data.dma_addr = new_dma_addr;
+
 	/* We can't fail anymore at this point: it's safe to unmap the skb. */
 	mtk_star_dma_unmap_rx(priv, &desc_data);
 
@@ -1318,7 +1320,6 @@ static int mtk_star_receive_packet(struct mtk_star_priv *priv)
 	netif_receive_skb(desc_data.skb);
 
 push_new_skb:
-	desc_data.dma_addr = new_dma_addr;
 	desc_data.len = skb_tailroom(new_skb);
 	desc_data.skb = new_skb;
 
-- 
2.25.0


_______________________________________________
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: [RFC PATCH 2/7] KVM: arm64: Set DBM bit of PTEs if hw DBM enabled
From: zhukeqian @ 2020-05-27  9:28 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Andrew Morton, kvm, Suzuki K Poulose, Marc Zyngier, linux-kernel,
	Sean Christopherson, Peng Liang, Alexios Zavras, zhengxiang9,
	Mark Brown, James Morse, Julien Thierry, wanghaibin.wang,
	Thomas Gleixner, Will Deacon, kvmarm, linux-arm-kernel
In-Reply-To: <20200526114926.GD17051@gaia>

Hi Catalin,

On 2020/5/26 19:49, Catalin Marinas wrote:
> On Mon, May 25, 2020 at 07:24:01PM +0800, Keqian Zhu wrote:
>> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
>> index 1305e28225fc..f9910ba2afd8 100644
>> --- a/arch/arm64/include/asm/pgtable-prot.h
>> +++ b/arch/arm64/include/asm/pgtable-prot.h
>> @@ -79,6 +79,7 @@ extern bool arm64_use_ng_mappings;
>>  	})
>>  
>>  #define PAGE_S2			__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PAGE_S2_XN)
>> +#define PAGE_S2_DBM		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PAGE_S2_XN | PTE_DBM)
> 
> You don't need a new page permission (see below).
> 
>>  #define PAGE_S2_DEVICE		__pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
>>  
>>  #define PAGE_NONE		__pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) | PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)
>> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
>> index e3b9ee268823..dc97988eb2e0 100644
>> --- a/virt/kvm/arm/mmu.c
>> +++ b/virt/kvm/arm/mmu.c
>> @@ -1426,6 +1426,10 @@ static void stage2_wp_ptes(pmd_t *pmd, phys_addr_t addr, phys_addr_t end)
>>  	pte = pte_offset_kernel(pmd, addr);
>>  	do {
>>  		if (!pte_none(*pte)) {
>> +#ifdef CONFIG_ARM64_HW_AFDBM
>> +			if (kvm_hw_dbm_enabled() && !kvm_s2pte_dbm(pte))
>> +				kvm_set_s2pte_dbm(pte);
>> +#endif
>>  			if (!kvm_s2pte_readonly(pte))
>>  				kvm_set_s2pte_readonly(pte);
>>  		}
> 
> Setting the DBM bit is equivalent to marking the page writable. The
> actual writable pte bit (S2AP[1] or HAP[2] as we call them in Linux for
> legacy reasons) tells you whether the page has been dirtied but it is
> still writable if you set DBM. Doing this in stage2_wp_ptes()
> practically means that you no longer have read-only pages at S2. There
> are several good reasons why you don't want to break this. For example,
> the S2 pte may already be read-only for other reasons (CoW).
> 
Thanks, your comments help to solve the first problem in cover letter.

> I think you should only set the DBM bit if the pte was previously
> writable. In addition, any permission change to the S2 pte must take
> into account the DBM bit and clear it while transferring the dirty
> status to the underlying page. I'm not deeply familiar with all these
> callbacks into KVM but two such paths are kvm_unmap_hva_range() and the
> kvm_mmu_notifier_change_pte().
Yes, I agree.
> 
> 
>> @@ -1827,7 +1831,15 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
>>  
>>  		ret = stage2_set_pmd_huge(kvm, memcache, fault_ipa, &new_pmd);
>>  	} else {
>> -		pte_t new_pte = kvm_pfn_pte(pfn, mem_type);
>> +		pte_t new_pte;
>> +
>> +#ifdef CONFIG_ARM64_HW_AFDBM
>> +		if (kvm_hw_dbm_enabled() &&
>> +		    pgprot_val(mem_type) == pgprot_val(PAGE_S2)) {
>> +			mem_type = PAGE_S2_DBM;
>> +		}
>> +#endif
>> +		new_pte = kvm_pfn_pte(pfn, mem_type);
>>  
>>  		if (writable) {
>>  			new_pte = kvm_s2pte_mkwrite(new_pte);
> 
> That's wrong here. Basically for any fault you get, you just turn the S2
> page writable. The point of DBM is that you don't get write faults at
> all if you have a writable page. So, as I said above, only set the DBM
> bit if you stored a writable S2 pte (kvm_s2pte_mkwrite()).
Yeah, you are right. I will correct it in Patch v1.
> 

Thanks,
Keqian


_______________________________________________
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 26/26] KVM: arm64: Parametrize exception entry with a target EL
From: Marc Zyngier @ 2020-05-27  9:34 UTC (permalink / raw)
  To: Mark Rutland
  Cc: kvm, Suzuki K Poulose, Jintack Lim, Andre Przywara,
	Christoffer Dall, kvmarm, Will Deacon, George Cherian,
	James Morse, Julien Thierry, Zengtao (B), Catalin Marinas,
	Alexandru Elisei, Dave Martin, linux-arm-kernel
In-Reply-To: <20200519104457.GA19548@C02TD0UTHF1T.local>

HI Mark,

On 2020-05-19 11:44, Mark Rutland wrote:
> On Wed, Apr 22, 2020 at 01:00:50PM +0100, Marc Zyngier wrote:
>> We currently assume that an exception is delivered to EL1, always.
>> Once we emulate EL2, this no longer will be the case. To prepare
>> for this, add a target_mode parameter.
>> 
>> While we're at it, merge the computing of the target PC and PSTATE in
>> a single function that updates both PC and CPSR after saving their
>> previous values in the corresponding ELR/SPSR. This ensures that they
>> are updated in the correct order (a pretty common source of bugs...).
>> 
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>  arch/arm64/kvm/inject_fault.c | 75 
>> ++++++++++++++++++-----------------
>>  1 file changed, 38 insertions(+), 37 deletions(-)
>> 
>> diff --git a/arch/arm64/kvm/inject_fault.c 
>> b/arch/arm64/kvm/inject_fault.c
>> index d3ebf8bca4b89..3dbcbc839b9c3 100644
>> --- a/arch/arm64/kvm/inject_fault.c
>> +++ b/arch/arm64/kvm/inject_fault.c
>> @@ -26,28 +26,12 @@ enum exception_type {
>>  	except_type_serror	= 0x180,
>>  };
>> 
>> -static u64 get_except_vector(struct kvm_vcpu *vcpu, enum 
>> exception_type type)
>> -{
>> -	u64 exc_offset;
>> -
>> -	switch (*vcpu_cpsr(vcpu) & (PSR_MODE_MASK | PSR_MODE32_BIT)) {
>> -	case PSR_MODE_EL1t:
>> -		exc_offset = CURRENT_EL_SP_EL0_VECTOR;
>> -		break;
>> -	case PSR_MODE_EL1h:
>> -		exc_offset = CURRENT_EL_SP_ELx_VECTOR;
>> -		break;
>> -	case PSR_MODE_EL0t:
>> -		exc_offset = LOWER_EL_AArch64_VECTOR;
>> -		break;
>> -	default:
>> -		exc_offset = LOWER_EL_AArch32_VECTOR;
>> -	}
>> -
>> -	return vcpu_read_sys_reg(vcpu, VBAR_EL1) + exc_offset + type;
>> -}
>> -
>>  /*
>> + * This performs the exception entry at a given EL (@target_mode), 
>> stashing PC
>> + * and PSTATE into ELR and SPSR respectively, and compute the new 
>> PC/PSTATE.
>> + * The EL passed to this function *must* be a non-secure, privileged 
>> mode with
>> + * bit 0 being set (PSTATE.SP == 1).
>> + *
>>   * When an exception is taken, most PSTATE fields are left unchanged 
>> in the
>>   * handler. However, some are explicitly overridden (e.g. M[4:0]). 
>> Luckily all
>>   * of the inherited bits have the same position in the 
>> AArch64/AArch32 SPSR_ELx
>> @@ -59,10 +43,35 @@ static u64 get_except_vector(struct kvm_vcpu 
>> *vcpu, enum exception_type type)
>>   * Here we manipulate the fields in order of the AArch64 SPSR_ELx 
>> layout, from
>>   * MSB to LSB.
>>   */
>> -static unsigned long get_except64_pstate(struct kvm_vcpu *vcpu)
>> +static void enter_exception(struct kvm_vcpu *vcpu, unsigned long 
>> target_mode,
>> +			    enum exception_type type)
> 
> Since this is all for an AArch64 target, could we keep `64` in the 
> name,
> e.g enter_exception64? That'd mirror the callers below.
> 
>>  {
>> -	unsigned long sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
>> -	unsigned long old, new;
>> +	unsigned long sctlr, vbar, old, new, mode;
>> +	u64 exc_offset;
>> +
>> +	mode = *vcpu_cpsr(vcpu) & (PSR_MODE_MASK | PSR_MODE32_BIT);
>> +
>> +	if      (mode == target_mode)
>> +		exc_offset = CURRENT_EL_SP_ELx_VECTOR;
>> +	else if ((mode | 1) == target_mode)
>> +		exc_offset = CURRENT_EL_SP_EL0_VECTOR;
> 
> It would be nice if we could add a mnemonic for the `1` here, e.g.
> PSR_MODE_SP0 or PSR_MODE_THREAD_BIT.

I've addressed both comments as follows:

diff --git a/arch/arm64/include/asm/ptrace.h 
b/arch/arm64/include/asm/ptrace.h
index bf57308fcd63..953b6a1ce549 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -35,6 +35,7 @@
  #define GIC_PRIO_PSR_I_SET		(1 << 4)

  /* Additional SPSR bits not exposed in the UABI */
+#define PSR_MODE_THREAD_BIT	(1 << 0)
  #define PSR_IL_BIT		(1 << 20)

  /* AArch32-specific ptrace requests */
diff --git a/arch/arm64/kvm/inject_fault.c 
b/arch/arm64/kvm/inject_fault.c
index 3dbcbc839b9c..ebfdfc27b2bd 100644
--- a/arch/arm64/kvm/inject_fault.c
+++ b/arch/arm64/kvm/inject_fault.c
@@ -43,8 +43,8 @@ enum exception_type {
   * Here we manipulate the fields in order of the AArch64 SPSR_ELx 
layout, from
   * MSB to LSB.
   */
-static void enter_exception(struct kvm_vcpu *vcpu, unsigned long 
target_mode,
-			    enum exception_type type)
+static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long 
target_mode,
+			      enum exception_type type)
  {
  	unsigned long sctlr, vbar, old, new, mode;
  	u64 exc_offset;
@@ -53,7 +53,7 @@ static void enter_exception(struct kvm_vcpu *vcpu, 
unsigned long target_mode,

  	if      (mode == target_mode)
  		exc_offset = CURRENT_EL_SP_ELx_VECTOR;
-	else if ((mode | 1) == target_mode)
+	else if ((mode | PSR_MODE_THREAD_BIT) == target_mode)
  		exc_offset = CURRENT_EL_SP_EL0_VECTOR;
  	else if (!(mode & PSR_MODE32_BIT))
  		exc_offset = LOWER_EL_AArch64_VECTOR;
@@ -126,7 +126,7 @@ static void inject_abt64(struct kvm_vcpu *vcpu, bool 
is_iabt, unsigned long addr
  	bool is_aarch32 = vcpu_mode_is_32bit(vcpu);
  	u32 esr = 0;

-	enter_exception(vcpu, PSR_MODE_EL1h, except_type_sync);
+	enter_exception64(vcpu, PSR_MODE_EL1h, except_type_sync);

  	vcpu_write_sys_reg(vcpu, addr, FAR_EL1);

@@ -156,7 +156,7 @@ static void inject_undef64(struct kvm_vcpu *vcpu)
  {
  	u32 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT);

-	enter_exception(vcpu, PSR_MODE_EL1h, except_type_sync);
+	enter_exception64(vcpu, PSR_MODE_EL1h, except_type_sync);

  	/*
  	 * Build an unknown exception, depending on the instruction


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 related

* Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone
From: Marco Felsch @ 2020-05-27  9:35 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: robh, kernel, Anson.Huang, devicetree, shawnguo, s.hauer, angus,
	linux-kernel, linux-imx, kernel, mchehab, festevam, agx,
	linux-arm-kernel
In-Reply-To: <20200514155737.12160-1-martin.kepplinger@puri.sm>

Hi Martin,

On 20-05-14 17:57, Martin Kepplinger wrote:
> From: "Angus Ainslie (Purism)" <angus@akkea.ca>
> 
> Add a devicetree description for the Librem 5 phone. The early batches
> that have been sold are supported as well as the mass-produced device
> available later this year, see https://puri.sm/products/librem-5/
> 
> This boots to a working console with working WWAN modem, wifi usdhc,
> IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> ---
>  arch/arm64/boot/dts/freescale/Makefile        |    1 +
>  .../boot/dts/freescale/imx8mq-librem5.dts     | 1174 +++++++++++++++++
>  2 files changed, 1175 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> 
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index cd38d04da5a7..342579121f98 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
>  dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> new file mode 100644
> index 000000000000..95c105b4c120
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> @@ -0,0 +1,1174 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018-2020 Purism SPC
> + */
> +
> +/dts-v1/;
> +
> +#include "dt-bindings/input/input.h"
> +#include "dt-bindings/pwm/pwm.h"
> +#include "dt-bindings/usb/pd.h"
> +#include "imx8mq.dtsi"
> +
> +/ {
> +	model = "Purism Librem 5";
> +	compatible = "purism,librem5", "fsl,imx8mq";
> +
> +	backlight_dsi: backlight-dsi {
> +		compatible = "led-backlight";
> +		leds = <&led_backlight>;
> +		brightness-levels = <255>;
> +		default-brightness-level = <100>;
> +	};
> +
> +	bm818_codec: sound-wwan-codec {
> +		compatible = "broadmobi,bm818", "option,gtm601";
> +		#sound-dai-cells = <0>;
> +	};

Please sort the node names alpabetical.

> +
> +	chosen {
> +		stdout-path = &uart1;
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_keys>, <&pinctrl_hp>;
> +
> +		hp-det {
> +			label = "HP_DET";
> +			gpios = <&gpio3 9 GPIO_ACTIVE_HIGH>;
> +			wakeup-source;
> +			linux,code = <KEY_HP>;

Nit: I would add the wakeup-source behind the linux,code.

> +		};
> +
> +		vol-down {
> +			label = "VOL_DOWN";
> +			gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEDOWN>;
> +		};
> +
> +		vol-up {
> +			label = "VOL_UP";
> +			gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_VOLUMEUP>;
> +		};
> +	};
> +
> +	pwmleds {
> +		compatible = "pwm-leds";
> +
> +		blue {
> +			label = "phone:blue:front";
> +			max-brightness = <248>;
> +			pwms = <&pwm2 0 50000>;
> +		};
> +
> +		green {
> +			label = "phone:green:front";
> +			max-brightness = <248>;
> +			pwms = <&pwm4 0 50000>;
> +		};
> +
> +		red {
> +			label = "phone:red:front";
> +			max-brightness = <248>;
> +			pwms = <&pwm3 0 50000>;
> +		};
> +	};
> +
> +	pmic_osc: clock-pmic {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "pmic_osc";
> +	};

Please sort nodes alphabetical.

> +
> +	reg_audio_pwr_en: regulator-audio-pwr-en {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_audiopwr>;
> +		regulator-name = "AUDIO_PWR_EN";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +		regulator-always-on;

Why should this regulator be always on? The wm8962.c driver can handle
the regualtor enable/disable.

> +	};
> +
> +	reg_aud_1v8: regulator-audio-v1v8 {
				^
		     regulator-audio-1v8?

> +		compatible = "regulator-fixed";
> +		regulator-name = "aud_1v8";

Is it intended to use capitalized and no-capitalized regulator-name's?

> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		vin-supply = <&reg_audio_pwr_en>;
> +	};

Can we squash regulator-audio-pwr-en and regulator-audio-v1v8?

> +
> +	reg_gnss: regulator-gnss {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gnsspwr>;
> +		regulator-name = "GNSS";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio3 12 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_hub: regulator-hub {
> +		compatible = "regulator-fixed";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_hub_pwr>;
> +		regulator-name = "HUB";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
> +		enable-active-high;
> +	};
> +
> +	reg_lcd_1v8: regulator-lcd-1v8 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "lcd_1v8";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_dsien>;
> +		vin-supply = <&reg_vdd_1v8>;
> +		enable-active-high;
> +		gpio = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> +	};

This regulator is never used.

> +
> +	reg_lcd_3v4: regulator-lcd-3v4 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "lcd_3v4";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_dsibiasen>;
> +		vin-supply = <&reg_vsys_3v4>;
> +		enable-active-high;
> +		gpio = <&gpio1 20 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_vdd_sen: regulator-vdd-sen {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_sen";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	reg_vdd_3v3: regulator-vdd-3v3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_3v3";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	reg_vdd_1v8: regulator-vdd-1v8 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vdd_1v8";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +	};
> +
> +	reg_vsys_3v4: regulator-vsys-3v4 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vsys_3v4";
> +		regulator-min-microvolt = <3400000>;
> +		regulator-max-microvolt = <3400000>;
> +		regulator-always-on;
> +	};
> +
> +	reg_3v3_wifi: regulator-3v3-wifi {
			^
	reg_wifi_3v3: regulator-wifi-3v3?

> +		compatible = "regulator-fixed";
> +		regulator-name = "3v3_wifi";
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +	};
> +
> +	sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "wm8962";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,widgets =
> +			"Headphone", "Headphone",
> +			"Microphone", "Headset Mic",
> +			"Microphone", "Digital Mic",
> +			"Speaker", "Speaker";
> +		simple-audio-card,routing =
> +			"Headphone", "HPOUTL",
> +			"Headphone", "HPOUTR",
> +			"Speaker", "SPKOUTL",
> +			"Speaker", "SPKOUTR",
> +			"Headset Mic", "MICBIAS",
> +			"IN3R", "Headset Mic",
> +			"DMICDAT", "Digital Mic";
> +		simple-audio-card,cpu {
> +			sound-dai = <&sai2>;
> +		};
> +		simple-audio-card,codec {
> +			sound-dai = <&codec>;
> +			clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
> +			frame-master;
> +			bitclock-master;
> +		};
> +	};
> +
> +	sound-wwan {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "MODEM";
> +		simple-audio-card,format = "i2s";
> +
> +		simple-audio-card,cpu {
> +			sound-dai = <&sai6>;
> +			frame-inversion;
> +		};
> +
> +		telephony_link_master: simple-audio-card,codec {
			^
		useless phandle?
> +			sound-dai = <&bm818_codec>;
> +			frame-master;
> +			bitclock-master;
> +		};
> +	};
> +
> +	vibrator {
> +		compatible = "pwm-vibrator";
> +		pwms = <&pwm1 0 1000000000 0>;
> +		pwm-names = "enable";
> +		vcc-supply = <&reg_vdd_3v3>;
> +	};
> +};
> +
> +&A53_0 {
> +	cpu-supply = <&buck2_reg>;
> +};
> +
> +&A53_1 {
> +	cpu-supply = <&buck2_reg>;
> +};
> +
> +&A53_2 {
> +	cpu-supply = <&buck2_reg>;
> +};
> +
> +&A53_3 {
> +	cpu-supply = <&buck2_reg>;
> +};
> +
> +&clk {
> +	assigned-clocks = <&clk IMX8MQ_AUDIO_PLL1>, <&clk IMX8MQ_AUDIO_PLL2>;
> +	assigned-clock-rates = <786432000>, <722534400>;
> +};

Either I would bundle all clock settings here or within the sai nodes.

> +
> +&ddrc {
> +	operating-points-v2 = <&ddrc_opp_table>;
> +
> +	ddrc_opp_table: ddrc-opp-table {
> +		compatible = "operating-points-v2";
> +
> +		opp-25M {
> +			opp-hz = /bits/ 64 <25000000>;
> +		};
> +		opp-100M {
> +			opp-hz = /bits/ 64 <100000000>;
> +		};
> +		opp-800M {
> +			opp-hz = /bits/ 64 <800000000>;
> +		};
> +	};
> +};
> +
> +&dphy {
> +	status = "okay";
> +};
> +
> +&ecspi1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_ecspi1>;
> +	cs-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>;

Missmatch with the pinctrl_ecspi1?

> +	status = "okay";

Status is always the last property.

> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	nor_flash: flash@0 {
> +		compatible = "jedec,spi-nor";
> +		spi-max-frequency = <1000000>;
> +		reg = <0>;
> +	};
> +};
> +
> +&gpio1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_pmic_5v>;
> +
> +	pmic-5v {
> +		gpio-hog;
> +		gpio = <&gpio1 1 GPIO_ACTIVE_HIGH>;
> +		input;
> +	};
> +};
> +
> +&iomuxc {
> +	pinctrl_audiopwr: audiopwrgrp {
> +		fsl,pins = <
> +			/* AUDIO_POWER_EN_3V3 */
> +			MX8MQ_IOMUXC_GPIO1_IO04_GPIO1_IO4	0x83
> +		>;
> +	};
> +
> +	pinctrl_bl: blgrp {
> +		fsl,pins = <
> +			/* BACKLINGE_EN */
> +			MX8MQ_IOMUXC_NAND_DQS_GPIO3_IO14	0x83
> +		>;
> +	};
> +
> +	pinctrl_charger_in: chargeringrp {
> +		fsl,pins = <
> +			/* CHRG_INT */
> +			MX8MQ_IOMUXC_NAND_CE2_B_GPIO3_IO3	0x80
> +			/* CHG_STATUS_B */
> +			MX8MQ_IOMUXC_NAND_ALE_GPIO3_IO0		0x80
> +		>;
> +	};
> +
> +	pinctrl_dsibiasen: dsibiasengrp {
> +		fsl,pins = <
> +			/* DSI_BIAS_EN */
> +			MX8MQ_IOMUXC_ENET_TD1_GPIO1_IO20	0x83
> +		>;
> +	};
> +
> +	pinctrl_dsien: dsiengrp {
> +		fsl,pins = <
> +			/* DSI_EN_3V3 */
> +			MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5	0x83
> +		>;
> +	};
> +
> +	pinctrl_ecspi1: spi1grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_ECSPI1_MOSI_ECSPI1_MOSI	0x83
> +			MX8MQ_IOMUXC_ECSPI1_MISO_ECSPI1_MISO	0x83
> +			MX8MQ_IOMUXC_ECSPI1_SS0_GPIO5_IO9	0x19
> +			MX8MQ_IOMUXC_ECSPI1_SCLK_ECSPI1_SCLK	0x83
> +			/* SPI_SS1 */
> +			MX8MQ_IOMUXC_UART4_RXD_GPIO5_IO28	0x19
> +		>;
> +	};
> +
> +	pinctrl_gauge: gaugegrp {
> +		fsl,pins = <
> +			/* BAT_LOW */
> +			MX8MQ_IOMUXC_SAI5_RXC_GPIO3_IO20	0x80
> +		>;
> +	};
> +
> +	pinctrl_gnsspwr: gnsspwrgrp {
> +		fsl,pins = <
> +			/* GPS3V3_EN */
> +			MX8MQ_IOMUXC_NAND_DATA06_GPIO3_IO12	0x83
> +		>;
> +	};
> +
> +	pinctrl_haptic: hapticgrp {
> +		fsl,pins = <
> +			/* MOTO */
> +			MX8MQ_IOMUXC_SPDIF_EXT_CLK_PWM1_OUT	0x83
> +		>;
> +	};
> +
> +	pinctrl_hp: hpgrp {
> +		fsl,pins = <
> +			/* HEADPHONE_DET_1V8 */
> +			MX8MQ_IOMUXC_NAND_DATA03_GPIO3_IO9	0x180
> +		>;
> +	};
> +
> +	pinctrl_hub_pwr: hubpwrgrp {
> +		fsl,pins = <
> +			/* HUB_PWR_3V3_EN */
> +			MX8MQ_IOMUXC_GPIO1_IO14_GPIO1_IO14	0x83
> +		>;
> +	};
> +
> +	pinctrl_i2c1: i2c1grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL		0x40000026
> +			MX8MQ_IOMUXC_I2C1_SDA_I2C1_SDA		0x40000026
> +		>;
> +	};
> +
> +	pinctrl_i2c2: i2c2grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_I2C2_SCL_I2C2_SCL		0x40000026
> +			MX8MQ_IOMUXC_I2C2_SDA_I2C2_SDA		0x40000026
> +		>;
> +	};
> +
> +	pinctrl_i2c3: i2c3grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_I2C3_SCL_I2C3_SCL		0x40000026
> +			MX8MQ_IOMUXC_I2C3_SDA_I2C3_SDA		0x40000026
> +		>;
> +	};
> +
> +	pinctrl_i2c4: i2c4grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL		0x40000026
> +			MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA		0x40000026
> +		>;
> +	};
> +
> +	pinctrl_keys: keysgrp {
> +		fsl,pins = <
> +			/* 4G_WAKE */
> +			MX8MQ_IOMUXC_NAND_RE_B_GPIO3_IO15	0x80
> +			/* PWR_KEY */
> +			MX8MQ_IOMUXC_NAND_CLE_GPIO3_IO5		0x01C0

gpio3 5/15 are never used was this intended?

> +			/* VOL- */
> +			MX8MQ_IOMUXC_ENET_MDIO_GPIO1_IO17	0x01C0
> +			/* VOL+ */
> +			MX8MQ_IOMUXC_ENET_MDC_GPIO1_IO16	0x01C0
> +		>;
> +	};
> +
> +	pinctrl_led_b: ledbgrp {
> +		fsl,pins = <
> +			/* LED_B */
> +			MX8MQ_IOMUXC_GPIO1_IO13_PWM2_OUT	0x06
> +		>;
> +	};
> +
> +	pinctrl_led_g: ledggrp {
> +		fsl,pins = <
> +			/* LED_G */
> +			MX8MQ_IOMUXC_SAI3_MCLK_PWM4_OUT		0x06
> +		>;
> +	};
> +
> +	pinctrl_led_r: ledrgrp {
> +		fsl,pins = <
> +			/* LED_R */
> +			MX8MQ_IOMUXC_SPDIF_TX_PWM3_OUT		0x06
> +		>;
> +	};
> +
> +	pinctrl_mag: maggrp {
> +		fsl,pins = <
> +			/* INT_MAG */
> +			MX8MQ_IOMUXC_SAI5_RXD1_GPIO3_IO22	0x80
> +		>;
> +	};
> +
> +	pinctrl_pmic: pmicgrp {
> +		fsl,pins = <
> +			/* PMIC_NINT */
> +			MX8MQ_IOMUXC_GPIO1_IO07_GPIO1_IO7	0x80
> +		>;
> +	};
> +
> +	pinctrl_pmic_5v: pmic5vgrp {
> +		fsl,pins = <
> +			/* PMIC_5V */
> +			MX8MQ_IOMUXC_GPIO1_IO01_GPIO1_IO1	0x80
> +		>;
> +	};
> +
> +	pinctrl_prox: proxgrp {
> +		fsl,pins = <
> +			/* INT_LIGHT */
> +			MX8MQ_IOMUXC_NAND_DATA01_GPIO3_IO7	0x80
> +		>;
> +	};
> +
> +	pinctrl_rtc: rtcgrp {
> +		fsl,pins = <
> +			/* RTC_INT */
> +			MX8MQ_IOMUXC_GPIO1_IO09_GPIO1_IO9	0x80
> +		>;
> +	};
> +
> +	pinctrl_sai2: sai2grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SAI2_TXD0_SAI2_TX_DATA0	0xd6
> +			MX8MQ_IOMUXC_SAI2_TXFS_SAI2_TX_SYNC	0xd6
> +			MX8MQ_IOMUXC_SAI2_MCLK_SAI2_MCLK	0xd6
> +			MX8MQ_IOMUXC_SAI2_RXD0_SAI2_RX_DATA0	0xd6
> +			MX8MQ_IOMUXC_SAI2_TXC_SAI2_TX_BCLK	0xd6
> +		>;
> +	};
> +
> +	pinctrl_sai6: sai6grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SAI1_RXD5_SAI6_RX_DATA0	0xd6
> +			MX8MQ_IOMUXC_SAI1_RXD6_SAI6_RX_SYNC	0xd6
> +			MX8MQ_IOMUXC_SAI1_TXD4_SAI6_RX_BCLK	0xd6
> +			MX8MQ_IOMUXC_SAI1_TXD5_SAI6_TX_DATA0	0xd6
> +		>;
> +	};
> +
> +	pinctrl_tcpc: tcpcgrp {
> +		fsl,pins = <
> +			/* TCPC_INT */
> +			MX8MQ_IOMUXC_GPIO1_IO10_GPIO1_IO10	0x01C0
> +		>;
> +	};
> +
> +	pinctrl_typec: typecgrp {
> +		fsl,pins = <
> +			/* TYPEC_MUX_EN */
> +			MX8MQ_IOMUXC_GPIO1_IO11_GPIO1_IO11	0x83
> +		>;
> +	};
> +
> +	pinctrl_uart1: uart1grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_UART1_RXD_UART1_DCE_RX	0x49
> +			MX8MQ_IOMUXC_UART1_TXD_UART1_DCE_TX	0x49
> +		>;
> +	};
> +
> +	pinctrl_uart2: uart2grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_UART2_TXD_UART2_DCE_TX	0x49
> +			MX8MQ_IOMUXC_UART2_RXD_UART2_DCE_RX	0x49
> +		>;
> +	};
> +
> +	pinctrl_uart3: uart3grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_UART3_RXD_UART3_DCE_RX	0x49
> +			MX8MQ_IOMUXC_UART3_TXD_UART3_DCE_TX	0x49
> +		>;
> +	};
> +
> +	pinctrl_uart4: uart4grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_ECSPI2_SCLK_UART4_DCE_RX		0x49
> +			MX8MQ_IOMUXC_ECSPI2_MOSI_UART4_DCE_TX		0x49
> +			MX8MQ_IOMUXC_ECSPI2_MISO_UART4_DCE_CTS_B	0x49
> +			MX8MQ_IOMUXC_ECSPI2_SS0_UART4_DCE_RTS_B		0x49
> +		>;
> +	};
> +
> +	pinctrl_usdhc1: usdhc1grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x83
> +			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xc3
> +			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xc3
> +			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xc3
> +			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xc3
> +			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xc3
> +			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xc3
> +			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xc3
> +			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xc3
> +			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xc3
> +			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x83
> +			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x8d
> +			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xcd
> +			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xcd
> +			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xcd
> +			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xcd
> +			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xcd
> +			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xcd
> +			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xcd
> +			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xcd
> +			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xcd
> +			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x8d
> +			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK			0x9f
> +			MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD			0xdf
> +			MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0		0xdf
> +			MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1		0xdf
> +			MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2		0xdf
> +			MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3		0xdf
> +			MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4		0xdf
> +			MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5		0xdf
> +			MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6		0xdf
> +			MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7		0xdf
> +			MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE		0x9f
> +			MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B		0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2: usdhc2grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B	0x80
> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK		0x83
> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD		0xc3
> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0	0xc3
> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1	0xc3
> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2	0xc3
> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3	0xc3
> +			MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B	0x80
> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK		0x8d
> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD		0xcd
> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0	0xcd
> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1	0xcd
> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2	0xcd
> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3	0xcd
> +			MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
> +		>;
> +	};
> +
> +	pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B	0x80
> +			MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK		0x9f
> +			MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD		0xcf
> +			MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0	0xcf
> +			MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1	0xcf
> +			MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2	0xcf
> +			MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3	0xcf
> +			MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
> +		>;
> +	};
> +
> +	pinctrl_wdog: wdoggrp {
> +		fsl,pins = <
> +			/* nWDOG */
> +			MX8MQ_IOMUXC_GPIO1_IO02_WDOG1_WDOG_B	0x1f
> +		>;
> +	};
> +};
> +
> +&i2c1 {
> +	clock-frequency = <387000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c1>;
> +	status = "okay";
> +
> +	typec_pd: usb-pd@3f {
> +		compatible = "ti,tps6598x";
> +		reg = <0x3f>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_typec>, <&pinctrl_tcpc>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
> +
> +		connector {
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				port@0 {
> +					reg = <0>;
> +
> +					usb_con_hs: endpoint {
> +						remote-endpoint = <&typec_hs>;
> +					};
> +				};
> +
> +				port@1 {
> +					reg = <1>;
> +
> +					usb_con_ss: endpoint {
> +						remote-endpoint = <&typec_ss>;
> +					};
> +				};
> +			};
> +		};
> +	};
> +
> +	pmic: pmic@4b {
> +		compatible = "rohm,bd71837";
> +		reg = <0x4b>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_pmic>;
> +		clocks = <&pmic_osc>;
> +		clock-names = "osc";
> +		clock-output-names = "pmic_clk";
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <7 GPIO_ACTIVE_LOW>;
> +		interrupt-names = "irq";
> +		rohm,reset-snvs-powered;
> +
> +		regulators {
> +			buck1_reg: BUCK1 {
> +				regulator-name = "buck1";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-ramp-delay = <1250>;
> +				rohm,dvs-run-voltage = <900000>;
> +				rohm,dvs-idle-voltage = <850000>;
> +				rohm,dvs-suspend-voltage = <800000>;
> +				regulator-always-on;
> +			};
> +
> +			buck2_reg: BUCK2 {
> +				regulator-name = "buck2";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1300000>;
> +				regulator-ramp-delay = <1250>;
> +				rohm,dvs-run-voltage = <1000000>;
> +				rohm,dvs-idle-voltage = <900000>;
> +				regulator-always-on;
> +			};
> +
> +			buck3_reg: BUCK3 {
> +				regulator-name = "buck3";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1300000>;
> +				rohm,dvs-run-voltage = <900000>;
> +				regulator-always-on;
> +			};
> +
> +			buck4_reg: BUCK4 {
> +				regulator-name = "buck4";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1300000>;
> +				rohm,dvs-run-voltage = <1000000>;
> +			};
> +
> +			buck5_reg: BUCK5 {
> +				regulator-name = "buck5";
> +				regulator-min-microvolt = <700000>;
> +				regulator-max-microvolt = <1350000>;
> +				regulator-always-on;
> +			};
> +
> +			buck6_reg: BUCK6 {
> +				regulator-name = "buck6";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +			};
> +
> +			buck7_reg: BUCK7 {
> +				regulator-name = "buck7";
> +				regulator-min-microvolt = <1605000>;
> +				regulator-max-microvolt = <1995000>;
> +				regulator-always-on;
> +			};
> +
> +			buck8_reg: BUCK8 {
> +				regulator-name = "buck8";
> +				regulator-min-microvolt = <800000>;
> +				regulator-max-microvolt = <1400000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo1_reg: LDO1 {
> +				regulator-name = "ldo1";
> +				regulator-min-microvolt = <3000000>;
> +				regulator-max-microvolt = <3300000>;
> +				/* leave on for snvs power button */
> +				regulator-always-on;
> +			};
> +
> +			ldo2_reg: LDO2 {
> +				regulator-name = "ldo2";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <900000>;
> +				/* leave on for snvs power button */
> +				regulator-always-on;
> +			};
> +
> +			ldo3_reg: LDO3 {
> +				regulator-name = "ldo3";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo4_reg: LDO4 {
> +				regulator-name = "ldo4";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo5_reg: LDO5 {
> +				/* VDD_PHY_0V9 - MIPI and HDMI domains */
> +				regulator-name = "ldo5";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo6_reg: LDO6 {
> +				/* VDD_PHY_0V9 - MIPI, HDMI and USB domains */
> +				regulator-name = "ldo6";
> +				regulator-min-microvolt = <900000>;
> +				regulator-max-microvolt = <1800000>;
> +				regulator-always-on;
> +			};
> +
> +			ldo7_reg: LDO7 {
> +				/* VDD_PHY_3V3 - USB domain */
> +				regulator-name = "ldo7";
> +				regulator-min-microvolt = <1800000>;
> +				regulator-max-microvolt = <3300000>;
> +				regulator-always-on;
> +			};

Out of curiosity, why did you marked all regulators as
regulator-always-on? I thought the librem5 is a smartphone.

> +		};
> +	};
> +
> +	rtc@68 {
> +		compatible = "microcrystal,rv4162";
> +		reg = <0x68>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_rtc>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +};
> +
> +&i2c2 {
> +	clock-frequency = <387000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c2>;
> +	status = "okay";
> +
> +	magnetometer@1e	{
> +		compatible = "st,lsm9ds1-magn";
> +		reg = <0x1e>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_mag>;
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
> +		vdd-supply = <&reg_vdd_sen>;
> +		vddio-supply = <&reg_vdd_1v8>;
> +	};
> +
> +	regulator@3e {
> +		compatible = "tps65132";
> +		reg = <0x3e>;
> +		reg_lcd_avdd: outp {
> +			regulator-name = "lcd_avdd";
> +			vin-supply = <&reg_lcd_3v4>;
> +		};
> +
> +		reg_lcd_avee: outn {
> +			regulator-name = "lcd_avee";
> +			vin-supply = <&reg_lcd_3v4>;
> +		};
		both phandles are not used.
> +	};
> +
> +	flash@53 {
> +		compatible = "lm3560";
> +		reg = <0x53>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		flash@0 {
> +			reg = <0x0>;
> +			flash-timeout-us = <150000>;
> +			flash-max-microamp = <320000>;
> +			led-max-microamp = <60000>;
> +			label = "lm3560:flash";
> +		};
> +
> +		torch@1 {
> +			reg = <0x1>;
> +			led-max-microamp = <10000>;
> +			label = "lm3560:torch";
> +		};
> +
> +	};
> +
> +	prox@60 {
> +		compatible = "vishay,vcnl4040";
> +		reg = <0x60>;
> +		pinctrl-0 = <&pinctrl_prox>;
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +
> +	accel-gyro@6a	{
> +		compatible = "st,lsm9ds1-imu";
> +		reg = <0x6a>;
> +		vdd-supply = <&reg_vdd_sen>;
> +		vddio-supply = <&reg_vdd_1v8>;
> +		mount-matrix =  "1",  "0",  "0",
> +				"0",  "1",  "0",
> +				"0",  "0", "-1";
> +	};
> +};
> +
> +&i2c3 {
> +	clock-frequency = <387000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c3>;
> +	status = "okay";
> +
> +	codec: wm8962@1a {

Please use generic names.

> +		compatible = "wlf,wm8962";
> +		reg = <0x1a>; // 0x4a is the test address
> +		clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
> +		assigned-clocks = <&clk IMX8MQ_CLK_SAI2>;
> +		assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
> +		assigned-clock-rates = <24576000>;
> +		#sound-dai-cells = <0>;
> +		mic-cfg = <0x200>;
> +		DCVDD-supply = <&reg_aud_1v8>;
> +		DBVDD-supply = <&reg_aud_1v8>;
> +		AVDD-supply = <&reg_aud_1v8>;
> +		CPVDD-supply = <&reg_aud_1v8>;
> +		MICVDD-supply = <&reg_aud_1v8>;
> +		PLLVDD-supply = <&reg_aud_1v8>;
> +		SPKVDD1-supply = <&reg_vsys_3v4>;
> +		SPKVDD2-supply = <&reg_vsys_3v4>;
> +		gpio-cfg = <
> +			0x0000 /* n/c */
> +			0x0001 /* gpio2, 1: default */
> +			0x0013 /* gpio3, 2: dmicclk */
> +			0x0000 /* n/c, 3: default */
> +			0x8014 /* gpio5, 4: dmic_dat */
> +			0x0000 /* gpio6, 5: default */
> +		>;
> +		status = "okay";

status can be dropped

> +	};
> +
> +	backlight@36 {
> +		compatible = "ti,lm36922";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_bl>;
> +		reg = <0x36>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		enable-gpios = <&gpio3 14 GPIO_ACTIVE_HIGH>;
> +		vled-supply = <&reg_vsys_3v4>;
> +		ti,ovp-microvolt = <25000000>;
> +
> +		led_backlight: led@0 {
> +			reg = <0>;
> +			label = "white:backlight_cluster";
> +			linux,default-trigger = "backlight";
> +			led-max-microamp = <20000>;
> +		};
> +	};
> +
> +	touchscreen@38 {
> +		compatible = "edt,edt-ft5506";
> +		reg = <0x38>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <27 IRQ_TYPE_EDGE_FALLING>;

You need to mux the irq gpio.

> +		irq-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;

irq-gpios is not supported by the driver. We only have a
wake/reset-gpio.

> +		touchscreen-size-x = <720>;
> +		touchscreen-size-y = <1440>;
> +	};
> +};
> +
> +&i2c4 {
> +	clock-frequency = <387000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c4>;
> +	status = "okay";
> +
> +	bat: fuel-gauge@36 {
> +		compatible = "maxim,max17055";
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_gauge>;
> +		reg = <0x36>;

Please check that "reg" is always the 2nd property after the
"compatible".

> +		maxim,over-heat-temp = <700>;
> +		maxim,over-volt = <4500>;
> +		maxim,rsns-microohm = <5000>;
> +	};
> +
> +	charger@6a { /* bq25895 */
> +		compatible = "ti,bq25890";

The compatible should be "ti,bq25895" if it is a bq25895. So we can drop
the comment too.

> +		reg = <0x6a>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_charger_in>;
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> +		phys = <&usb3_phy0>;
> +		ti,battery-regulation-voltage = <4192000>; /* 4.192V */
> +		ti,charge-current = <1600000>; /* 1.6A */
> +		ti,termination-current = <66000>;  /* 66mA */
> +		ti,precharge-current = <130000>; /* 130mA */
> +		ti,minimum-sys-voltage = <3700000>; /* 3.7V */
> +		ti,boost-voltage = <5000000>; /* 5V */
> +		ti,boost-max-current = <50000>; /* 50mA */
> +		ti,use-vinmin-threshold = <1>; /* enable VINDPM */
> +		ti,vinmin-threshold = <3900000>; /* 3.9V */

I would only mention the units within a comment because comments like
this begin to divergence after you fix something.

Regards,
  Marco

> +		monitored-battery = <&bat>;
> +	};
> +};
> +
> +&pgc_gpu {
> +	power-supply = <&buck3_reg>;
> +};
> +
> +&pgc_mipi {
> +	power-supply = <&ldo5_reg>;
> +};
> +
> +&pgc_vpu {
> +	power-supply = <&buck4_reg>;
> +};
> +
> +&pwm1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_haptic>;
> +	status = "okay";
> +};
> +
> +&pwm2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_led_b>;
> +	status = "okay";
> +};
> +
> +&pwm3 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_led_g>;
> +	status = "okay";
> +};
> +
> +&pwm4 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_led_r>;
> +	status = "okay";
> +};
> +
> +&sai2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai2>;
> +	assigned-clocks = <&clk IMX8MQ_CLK_SAI2>;
> +	assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
> +	assigned-clock-rates = <24576000>;
> +	status = "okay";
> +};
> +
> +&sai6 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sai6>;
> +	assigned-clocks = <&clk IMX8MQ_CLK_SAI6>;
> +	assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
> +	assigned-clock-rates = <24576000>;
> +	fsl,sai-synchronous-rx;
> +	status = "okay";
> +};
> +
> +&snvs_pwrkey {
> +	status = "okay";
> +};
> +
> +&snvs_rtc {
> +	status = "disabled";
> +};
> +
> +&uart1 { /* console */
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart1>;
> +	status = "okay";
> +};
> +
> +&uart2 { /* TPS - GPS - DEBUG */
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart2>;
> +	status = "okay";
> +
> +	gnss {
> +		compatible = "globaltop,pa6h";
> +		vcc-supply = <&reg_gnss>;
> +		current-speed = <9600>;
> +	};
> +};
> +
> +&uart3 { /* SMC */
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart3>;
> +	status = "okay";
> +};
> +
> +&uart4 { /* BT */
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_uart4>;
> +	uart-has-rtscts;
> +	status = "okay";
> +};
> +
> +&usb3_phy0 {
> +	status = "okay";
> +};
> +
> +&usb3_phy1 {
> +	vbus-supply = <&reg_hub>;
> +	status = "okay";
> +};
> +
> +&usb_dwc3_0 {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	dr_mode = "otg";
> +	snps,dis_u3_susphy_quirk;
> +	status = "okay";
> +
> +	port@0 {
> +		reg = <0>;
> +
> +		typec_hs: endpoint {
> +			remote-endpoint = <&usb_con_hs>;
> +		};
> +	};
> +
> +	port@1 {
> +		reg = <1>;
> +
> +		typec_ss: endpoint {
> +			remote-endpoint = <&usb_con_ss>;
> +		};
> +	};
> +};
> +
> +&usb_dwc3_1 {
> +	dr_mode = "host";
> +	snps,dis_u2_susphy_quirk;
> +	snps,dis_u3_susphy_quirk;
> +	status = "okay";
> +};
> +
> +&usdhc1 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc1>;
> +	pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
> +	bus-width = <8>;
> +	vmmc-supply = <&reg_vdd_3v3>;
> +	power-supply = <&reg_vdd_1v8>;
> +	non-removable;
> +	status = "okay";
> +};
> +
> +&usdhc2 {
> +	pinctrl-names = "default", "state_100mhz", "state_200mhz";
> +	pinctrl-0 = <&pinctrl_usdhc2>;
> +	pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
> +	pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
> +	bus-width = <4>;
> +	vmmc-supply = <&reg_3v3_wifi>;
> +	broken-cd;
> +	disable-wp;
> +	cap-sdio-irq;
> +	keep-power-in-suspend;
> +	wakeup-source;
> +	status = "okay";
> +};
> +
> +&wdog1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_wdog>;
> +	fsl,ext-reset-output;
> +	status = "okay";
> +};
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


_______________________________________________
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 0/4] PCI, iommu: Factor 'untrusted' check for ATS enablement
From: Jean-Philippe Brucker @ 2020-05-27  9:38 UTC (permalink / raw)
  To: linux-pci, linux-arm-kernel, iommu, joro, bhelgaas
  Cc: ashok.raj, will, hch, alex.williamson, robin.murphy, dwmw2,
	baolu.lu
In-Reply-To: <20200520152201.3309416-1-jean-philippe@linaro.org>

Hi Joerg, Bjorn,

On Wed, May 20, 2020 at 05:21:59PM +0200, Jean-Philippe Brucker wrote:
> IOMMU drivers currently check themselves if a device is untrusted
> (plugged into an external-facing port) before enabling ATS. Move the
> check to drivers/pci. The only functional change should be to the AMD
> IOMMU driver. With this change all IOMMU drivers block 'Translated' PCIe
> transactions and Translation Requests from untrusted devices.

This seems ready for v5.8. I guess it could go through the IOMMU tree
since there are a little more IOMMU changes?

Thanks,
Jean
> 
> Since v1 [1] I added tags, addressed comments on patches 1 and 3, and
> fixed a regression in patch 3.
> 
> [1] https://lore.kernel.org/linux-iommu/20200515104359.1178606-1-jean-philippe@linaro.org/
> 
> Jean-Philippe Brucker (4):
>   PCI/ATS: Only enable ATS for trusted devices
>   iommu/amd: Use pci_ats_supported()
>   iommu/arm-smmu-v3: Use pci_ats_supported()
>   iommu/vt-d: Use pci_ats_supported()
> 
>  include/linux/pci-ats.h     |  3 +++
>  drivers/iommu/amd_iommu.c   | 12 ++++--------
>  drivers/iommu/arm-smmu-v3.c | 20 +++++++-------------
>  drivers/iommu/intel-iommu.c |  9 +++------
>  drivers/pci/ats.c           | 18 +++++++++++++++++-
>  5 files changed, 34 insertions(+), 28 deletions(-)
> 
> -- 
> 2.26.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 2/2] MAINTAINERS: Add Purism Librem 5 section to the list
From: Marco Felsch @ 2020-05-27  9:40 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: robh, kernel, Anson.Huang, devicetree, shawnguo, s.hauer, angus,
	linux-kernel, linux-imx, kernel, mchehab, festevam, agx,
	linux-arm-kernel
In-Reply-To: <20200514155737.12160-2-martin.kepplinger@puri.sm>

On 20-05-14 17:57, Martin Kepplinger wrote:
> Add development information for the devicetree files for hardware
> by Purism SPC.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0abba1aff1ae..176efec31010 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13775,6 +13775,14 @@ T:	git git://linuxtv.org/media_tree.git
>  F:	Documentation/admin-guide/media/pulse8-cec.rst
>  F:	drivers/media/cec/usb/pulse8/
>  
> +PURISM LIBREM 5
> +M:	Purism Kernel Team <kernel@puri.sm>
> +S:	Supported
> +B:	https://source.puri.sm/Librem5/linux-next/issues
> +T:	https://source.puri.sm/Librem5/linux-next
> +F:	arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
> +F:	arch/arm64/boot/dts/freescale/imx8mq-librem5.dts

Is it okay to take care of all imx8mq-librem5* files? 

F:	arch/arm64/boot/dts/freescale/imx8mq-librem5*

Regards,
  Marco

> +
>  PVRUSB2 VIDEO4LINUX DRIVER
>  M:	Mike Isely <isely@pobox.com>
>  L:	pvrusb2@isely.net	(subscribers-only)
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
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] drm/mxsfb: Call drm_crtc_vblank_on/off
From: Daniel Vetter @ 2020-05-27  9:47 UTC (permalink / raw)
  To: DRI Development
  Cc: Marek Vasut, Fabio Estevam, Daniel Vetter,
	Intel Graphics Development, Stefan Agner, NXP Linux Team,
	Pengutronix Kernel Team, Daniel Vetter, Shawn Guo, Sascha Hauer,
	linux-arm-kernel

mxsfb has vblank support, is atomic, but doesn't call
drm_crtc_vblank_on/off as it should. Not good.

With my next patch to add the drm_crtc_vblank_reset to helpers this
means not even the very first crtc enabling will vblanks work anymore,
since they'll just stay off forever.

Since mxsfb doesn't have any vblank waits of its own in the
enable/disable flow, nor an enable/disable_vblank callback we can do
the on/off as the first respectively last operation, and it should all
work.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index 497cf443a9af..1891cd6deb2f 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -124,6 +124,7 @@ static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe,
 	drm_panel_prepare(mxsfb->panel);
 	mxsfb_crtc_enable(mxsfb);
 	drm_panel_enable(mxsfb->panel);
+	drm_crtc_vblank_on(&pipe->crtc);
 }
 
 static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe)
@@ -133,6 +134,7 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe)
 	struct drm_crtc *crtc = &pipe->crtc;
 	struct drm_pending_vblank_event *event;
 
+	drm_crtc_vblank_off(&pipe->crtc);
 	drm_panel_disable(mxsfb->panel);
 	mxsfb_crtc_disable(mxsfb);
 	drm_panel_unprepare(mxsfb->panel);
-- 
2.26.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 2/3] drivers: input: keyboard: Add mtk keypad driver
From: Andy Shevchenko @ 2020-05-27  9:50 UTC (permalink / raw)
  To: Marco Felsch
  Cc: linux-mediatek, Fengping Yu, Dmitry Torokhov, linux-arm-kernel,
	linux-input
In-Reply-To: <20200527090048.wawdb5qkkhroyjtf@pengutronix.de>

On Wed, May 27, 2020 at 11:00:48AM +0200, Marco Felsch wrote:
> On 20-05-27 16:31, Fengping Yu wrote:

> > +	irqnr = platform_get_irq(pdev, 0);
> > +	if (irqnr < 0)
> > +		return -irqnr;
> 		       ^
> 		       irqnr is already negative
> 
> > +	ret = devm_request_threaded_irq(&pdev->dev, irqnr,
> > +					NULL, kpd_irq_handler, 0,
> > +					MTK_KPD_NAME, keypad);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Failed to request IRQ#%d:%d\n",
> > +						irqnr, ret);
> 
> Still not alligned.
> 
> > +		return ret;
> > +	}

And on top, can't we use irq instead of irqnr?

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
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 v6 8/9] spi: stm32: Add 'SPI_SIMPLEX_RX', 'SPI_3WIRE_RX' support for stm32f4
From: Mark Brown @ 2020-05-27  9:51 UTC (permalink / raw)
  To: dillon.minfei
  Cc: devicetree, noralf, dillonhua, p.zabel, linux-clk, airlied,
	mturquette, linux-stm32, linux-kernel, dri-devel, linux-spi,
	sboyd, andy.shevchenko, robh+dt, thierry.reding, mcoquelin.stm32,
	daniel, sam, linus.walleij, linux-arm-kernel, alexandre.torgue
In-Reply-To: <1590564453-24499-9-git-send-email-dillon.minfei@gmail.com>


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

On Wed, May 27, 2020 at 03:27:32PM +0800, dillon.minfei@gmail.com wrote:
> From: dillon min <dillon.minfei@gmail.com>
> 
> in l3gd20 driver startup, there is a setup failed error return from
> stm32 spi driver

Please do not submit new versions of already applied patches, please
submit incremental updates to the existing code.  Modifying existing
commits creates problems for other users building on top of those
commits so it's best practice to only change pubished git commits if
absolutely essential.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 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 0/2] Introduce PCI_FIXUP_IOMMU
From: Arnd Bergmann @ 2020-05-27  9:53 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: jean-philippe, Lorenzo Pieralisi, Herbert Xu, linux-pci,
	Joerg Roedel, Hanjun Guo, Rafael J. Wysocki,
	linux-kernel@vger.kernel.org, open list:IOMMU DRIVERS,
	kenneth-lee-2012, ACPI Devel Maling List, Wangzhou,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Sudeep Holla,
	Bjorn Helgaas, Zhangfei Gao, Linux ARM, Len Brown
In-Reply-To: <20200527090007.GA179718@kroah.com>

On Wed, May 27, 2020 at 11:00 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, May 26, 2020 at 07:49:07PM +0800, Zhangfei Gao wrote:
> > Some platform devices appear as PCI but are actually on the AMBA bus,
>
> Why would these devices not just show up on the AMBA bus and use all of
> that logic instead of being a PCI device and having to go through odd
> fixes like this?

There is a general move to having hardware be discoverable even with
ARM processors. Having on-chip devices be discoverable using PCI config
space is how x86 SoCs usually do it, and that is generally a good thing
as it means we don't need to describe them in DT

I guess as the hardware designers are still learning about it, this is not
always done correctly. In general, we can also describe PCI devices on
DT and do fixups during the probing there, but I suspect that won't work
as easily using ACPI probing, so the fixup is keyed off the hardware ID,
again as is common for x86 on-chip devices.

      Arnd

_______________________________________________
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 v4 24/26] arm64: mte: Introduce early param to disable MTE support
From: Will Deacon @ 2020-05-27  9:55 UTC (permalink / raw)
  To: Patrick Daly
  Cc: linux-arch, Vladimir Murzin, Szabolcs Nagy, Catalin Marinas,
	Kevin Brodsky, linux-mm, Andrey Konovalov, Vincenzo Frascino,
	Peter Collingbourne, Dave P Martin, linux-arm-kernel
In-Reply-To: <20200527021153.GA24439@pdaly-linux.qualcomm.com>

On Tue, May 26, 2020 at 07:11:53PM -0700, Patrick Daly wrote:
> On Fri, May 22, 2020 at 11:37:15AM +0100, Catalin Marinas wrote:
> > On Thu, May 21, 2020 at 10:57:10PM -0700, Patrick Daly wrote:
> > > On Mon, May 18, 2020 at 06:20:55PM +0100, Catalin Marinas wrote:
> > > > On Mon, May 18, 2020 at 12:31:03PM +0100, Will Deacon wrote:
> > > > > On Mon, May 18, 2020 at 12:26:30PM +0100, Vladimir Murzin wrote:
> > > > > > On 5/15/20 6:16 PM, Catalin Marinas wrote:
> > > > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > > > > > index f2a93c8679e8..7436e7462b85 100644
> > > > > > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > > > > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > > > > > @@ -373,6 +373,10 @@
> > > > > > >  	arcrimi=	[HW,NET] ARCnet - "RIM I" (entirely mem-mapped) cards
> > > > > > >  			Format: <io>,<irq>,<nodeID>
> > > > > > >  
> > > > > > > +	arm64.mte_disable=
> > > > > > > +			[ARM64] Disable Linux support for the Memory
> > > > > > > +			Tagging Extension (both user and in-kernel).
> > > > > > > +
> > > > > > 
> > > > > > Should it really to take parameter (on/off/true/false)? It may lead to expectation
> > > > > > that arm64.mte_disable=false should enable MT and, yes, double negatives make it
> > > > > > look ugly, so if we do need parameter, can it be arm64.mte=on/off/true/false?
> > > > > 
> > > > > I don't think "performance analysis" is a good justification for this
> > > > > parameter tbh. We don't tend to add these options for other architectural
> > > > > features, and I don't see why MTE is any different in this regard.
> > > > 
> > > > There is an expectation of performance impact with MTE enabled,
> > > > especially if it's running in synchronous mode. For the in-kernel MTE,
> > > > we could add a parameter which sets sync vs async at boot time rather
> > > > than a big disable knob. It won't affect user space however.
> > > > 
> > > > The other 'justification' is if your hardware has weird unexpected
> > > > behaviour but I'd like this handled via errata workarounds.
> > > > 
> > > > I'll let the people who asked for this to chip in ;). I agree with you
> > > > that we rarely add these (and I rejected a similar option a few weeks
> > > > ago on the AMU patchset).
> > > 
> > > We've been looking into other ways this on/off behavior could be achieved.
> > 
> > The actual question here is what the on/off behaviour is needed for. We
> > can figure out the best mechanism for this once we know what we want to
> > achieve. My wild guess above was performance analysis but that can be
> > toggled by either kernel boot parameter or run-time sysctl (or just the
> > Kconfig option).
> > 
> > If it is about forcing user space not to use MTE, we may look into some
> > other sysctl controls (we already have one for the tagged address ABI).
> 
> We want to allow the end user to be able to easily "opt out" of MTE in favour
> of better power, perf and battery life.

Who is "the end user" in this case?

If MTE is bad enough for power, performance and battery life that we need a
kill switch, then perhaps we shouldn't enable it by default and the few
people that want to use it can build a kernel with it enabled. However, then
I don't really see what MTE buys you over the existing KASAN implementations.

I thought the general idea was that you could run in the (cheap) "async"
mode, and then re-run in the more expensive "sync" mode to further diagnose
any failures. That model seems to work well with these patches, since
reporting is disabled by default. Are you saying that there is a
significant penalty incurred even when reporting is not enabled?

Anyway, we don't offer global runtime/cmdline switches for the vast majority
of other architectural features -- instead, we choose a sensible default,
and I think we should do the same here.

Will

_______________________________________________
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