All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 5/5] lib: logic_pio: Enforce LOGIC_PIO_INDIRECT region ops are set at registration
From: John Garry @ 2019-06-20 10:31 UTC (permalink / raw)
  To: xuwei5; +Cc: bhelgaas, linuxarm, arm, linux-kernel, linux-pci, joe, John Garry
In-Reply-To: <1561026716-140537-1-git-send-email-john.garry@huawei.com>

Since the only LOGIC_PIO_INDIRECT host (hisi-lpc) now sets the ops prior
to registration, enforce this check at registration instead of in the IO
port accessors to simplify and marginally optimise the code.

A slight misalignment is also tidied.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 lib/logic_pio.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/logic_pio.c b/lib/logic_pio.c
index 45eb57af2574..126593ed9049 100644
--- a/lib/logic_pio.c
+++ b/lib/logic_pio.c
@@ -39,7 +39,8 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
 	resource_size_t iio_sz = MMIO_UPPER_LIMIT;
 	int ret = 0;
 
-	if (!new_range || !new_range->fwnode || !new_range->size)
+	if (!new_range || !new_range->fwnode || !new_range->size ||
+	    (new_range->flags == LOGIC_PIO_INDIRECT && !new_range->ops))
 		return -EINVAL;
 
 	start = new_range->hw_start;
@@ -237,7 +238,7 @@ type logic_in##bw(unsigned long addr)					\
 	} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) { \
 		struct logic_pio_hwaddr *entry = find_io_range(addr);	\
 									\
-		if (entry && entry->ops)				\
+		if (entry)						\
 			ret = entry->ops->in(entry->hostdata,		\
 					addr, sizeof(type));		\
 		else							\
@@ -253,7 +254,7 @@ void logic_out##bw(type value, unsigned long addr)			\
 	} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {	\
 		struct logic_pio_hwaddr *entry = find_io_range(addr);	\
 									\
-		if (entry && entry->ops)				\
+		if (entry)						\
 			entry->ops->out(entry->hostdata,		\
 					addr, value, sizeof(type));	\
 		else							\
@@ -261,7 +262,7 @@ void logic_out##bw(type value, unsigned long addr)			\
 	}								\
 }									\
 									\
-void logic_ins##bw(unsigned long addr, void *buffer,		\
+void logic_ins##bw(unsigned long addr, void *buffer,			\
 		   unsigned int count)					\
 {									\
 	if (addr < MMIO_UPPER_LIMIT) {					\
@@ -269,7 +270,7 @@ void logic_ins##bw(unsigned long addr, void *buffer,		\
 	} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {	\
 		struct logic_pio_hwaddr *entry = find_io_range(addr);	\
 									\
-		if (entry && entry->ops)				\
+		if (entry)						\
 			entry->ops->ins(entry->hostdata,		\
 				addr, buffer, sizeof(type), count);	\
 		else							\
@@ -286,7 +287,7 @@ void logic_outs##bw(unsigned long addr, const void *buffer,		\
 	} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {	\
 		struct logic_pio_hwaddr *entry = find_io_range(addr);	\
 									\
-		if (entry && entry->ops)				\
+		if (entry)						\
 			entry->ops->outs(entry->hostdata,		\
 				addr, buffer, sizeof(type), count);	\
 		else							\
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] spi/acpi: fix incorrect ACPI parent check
From: Ard Biesheuvel @ 2019-06-20 10:33 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Jarkko Nikula, linux-spi, Mark Brown, kbuild test robot,
	Dan Carpenter, Andy Shevchenko, Masahisa Kojima,
	Rafael J. Wysocki, ACPI Devel Maling List, Lukas Wunner
In-Reply-To: <20190619144255.GG2640@lahna.fi.intel.com>

On Wed, 19 Jun 2019 at 16:43, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Wed, Jun 19, 2019 at 05:17:59PM +0300, Jarkko Nikula wrote:
> > Hi
> >
> > On 6/19/19 4:58 PM, Ard Biesheuvel wrote:
> > > So can you explain how exactly the I2C GPIO expander is failing? I
> > > struggle to understand how the SPI slave probing could be related to
> > > that.
> > >
> > They don't show up in /sys/kernel/debug/gpio, are not present in
> > /sys/bus/i2c/devices/ but SPI core instead tries add them with a bogus Chip
> > Select number:
> >
> > [    5.727699][    T1] pxa2xx-spi pxa2xx-spi.5: cs56 >= max 4
> > [    5.733545][    T1] spi_master spi5: failed to add SPI device INT3491:00
> > from ACPI
>
> Just a guess but looking at the 4c3c59544f33 acpi_register_spi_device()
> does not seem to zero fill the whole struct acpi_spi_lookup structure so
> when it is supposed to bail out when SPI slave was not found:
>
>         if (!lookup.max_speed_hz)
>                 return AE_OK
>
> it instead continues to register SPI slave because lookup.max_speed_hz
> may contain whatever garbage there is in the stack at that address.

Good point.

Jarkko, does this help?

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50d230b33c42..d072efdd65ba 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1914,6 +1914,7 @@ static acpi_status
acpi_register_spi_device(struct spi_controller *ctlr,
                return AE_OK;

        lookup.ctlr             = ctlr;
+       lookup.max_speed_hz     = 0;
        lookup.mode             = 0;
        lookup.bits_per_word    = 0;
        lookup.irq              = -1;

^ permalink raw reply related

* [PATCH 3/5] bus: hisi_lpc: Unregister logical PIO range to avoid potential use-after-free
From: John Garry @ 2019-06-20 10:31 UTC (permalink / raw)
  To: xuwei5; +Cc: bhelgaas, linuxarm, arm, linux-kernel, linux-pci, joe, John Garry
In-Reply-To: <1561026716-140537-1-git-send-email-john.garry@huawei.com>

If, after registering a logical PIO range, the driver probe later fails,
the logical PIO range memory will be released automatically.

This causes an issue, in that the logical PIO range is not unregistered
and the released range memory may be later referenced.

Fix by unregistering the logical PIO range.

And since we now unregister the logical PIO range for probe failure, avoid
the special ordering of setting logical PIO range ops, which was a
previous (poor) attempt at a safeguard against this.

Fixes: adf38bb0b595 ("HISI LPC: Support the LPC host on Hip06/Hip07 with DT bindings")
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/bus/hisi_lpc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 19d7b6ff2f17..6d301aafcad2 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -606,24 +606,25 @@ static int hisi_lpc_probe(struct platform_device *pdev)
 	range->fwnode = dev->fwnode;
 	range->flags = LOGIC_PIO_INDIRECT;
 	range->size = PIO_INDIRECT_SIZE;
+	range->hostdata = lpcdev;
+	range->ops = &hisi_lpc_ops;
+	lpcdev->io_host = range;
 
 	ret = logic_pio_register_range(range);
 	if (ret) {
 		dev_err(dev, "register IO range failed (%d)!\n", ret);
 		return ret;
 	}
-	lpcdev->io_host = range;
 
 	/* register the LPC host PIO resources */
 	if (acpi_device)
 		ret = hisi_lpc_acpi_probe(dev);
 	else
 		ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
-	if (ret)
+	if (ret) {
+		logic_pio_unregister_range(range);
 		return ret;
-
-	lpcdev->io_host->hostdata = lpcdev;
-	lpcdev->io_host->ops = &hisi_lpc_ops;
+	}
 
 	io_end = lpcdev->io_host->io_start + lpcdev->io_host->size;
 	dev_info(dev, "registered range [%pa - %pa]\n",
-- 
2.17.1


^ permalink raw reply related

* [PATCH 4/5] bus: hisi_lpc: Add .remove method to avoid driver unbind crash
From: John Garry @ 2019-06-20 10:31 UTC (permalink / raw)
  To: xuwei5; +Cc: bhelgaas, linuxarm, arm, linux-kernel, linux-pci, joe, John Garry
In-Reply-To: <1561026716-140537-1-git-send-email-john.garry@huawei.com>

The original driver author seemed to be under the impression that a driver
cannot be removed if it does not have a .remove method. Or maybe if it is
a built-in platform driver.

This is not true. This crash can be created:

root@ubuntu:/sys/bus/platform/drivers/hisi-lpc# echo HISI0191\:00 > unbind
root@ubuntu:/sys/bus/platform/drivers/hisi-lpc# ipmitool raw 6 1
 Unable to handle kernel paging request at virtual address ffff000010035010
 Mem abort info:
   ESR = 0x96000047
   Exception class = DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
 Data abort info:
   ISV = 0, ISS = 0x00000047
   CM = 0, WnR = 1
 swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000000118b000
 [ffff000010035010] pgd=0000041ffbfff003, pud=0000041ffbffe003, pmd=0000041ffbffd003, pte=0000000000000000
 Internal error: Oops: 96000047 [#1] PREEMPT SMP
 Modules linked in:
 CPU: 17 PID: 1473 Comm: ipmitool Not tainted 5.2.0-rc5-00003-gf68c53b414a3-dirty #198
 Hardware name: Huawei Taishan 2280 /D05, BIOS Hisilicon D05 IT21 Nemo 2.0 RC0 04/18/2018
 pstate: 20000085 (nzCv daIf -PAN -UAO)
 pc : hisi_lpc_target_in+0x7c/0x120
 lr : hisi_lpc_target_in+0x70/0x120
 sp : ffff00001efe3930
 x29: ffff00001efe3930 x28: ffff841f9f599200
 x27: 0000000000000002 x26: 0000000000000000
 x25: 0000000000000080 x24: 00000000000000e4
 x23: 0000000000000000 x22: 0000000000000064
 x21: ffff801fb667d280 x20: 0000000000000001
 x19: ffff00001efe39ac x18: 0000000000000000
 x17: 0000000000000000 x16: 0000000000000000
 x15: 0000000000000000 x14: 0000000000000000
 x13: 0000000000000000 x12: 0000000000000000
 x11: 0000000000000000 x10: 0000000000000000
 x9 : 0000000000000000 x8 : ffff841febe60340
 x7 : ffff801fb55c52e8 x6 : 0000000000000000
 x5 : 0000000000ffc0e3 x4 : 0000000000000001
 x3 : ffff801fb667d280 x2 : 0000000000000001
 x1 : ffff000010035010 x0 : ffff000010035000
 Call trace:
  hisi_lpc_target_in+0x7c/0x120
  hisi_lpc_comm_in+0x88/0x98
  logic_inb+0x5c/0xb8
  port_inb+0x18/0x20
  bt_event+0x38/0x808
  smi_event_handler+0x4c/0x5a0
  check_start_timer_thread.part.4+0x40/0x58
  sender+0x78/0x88
  smi_send.isra.6+0x94/0x108
  i_ipmi_request+0x2c4/0x8f8
  ipmi_request_settime+0x124/0x160
  handle_send_req+0x19c/0x208
  ipmi_ioctl+0x2c0/0x990
  do_vfs_ioctl+0xb8/0x8f8
  ksys_ioctl+0x80/0xb8
  __arm64_sys_ioctl+0x1c/0x28
  el0_svc_common.constprop.0+0x64/0x160
  el0_svc_handler+0x28/0x78
  el0_svc+0x8/0xc
 Code: 941d1511 aa0003f9 f94006a0 91004001 (b9000034)
 ---[ end trace aa842b86af7069e4 ]---

The problem here is that the host goes away but the associated logical PIO
region remains registered, as do the child devices.

Fix by adding a .remove method to tidy-up by removing the child devices
and unregistering the logical PIO region.

Fixes: adf38bb0b595 ("HISI LPC: Support the LPC host on Hip06/Hip07 with DT bindings")
Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/bus/hisi_lpc.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 6d301aafcad2..0e9f1f141c93 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -456,6 +456,17 @@ struct hisi_lpc_acpi_cell {
 	size_t pdata_size;
 };
 
+static void hisi_lpc_acpi_remove(struct device *hostdev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(hostdev);
+	struct acpi_device *child;
+
+	device_for_each_child(hostdev, NULL, hisi_lpc_acpi_remove_subdev);
+
+	list_for_each_entry(child, &adev->children, node)
+		acpi_device_clear_enumerated(child);
+}
+
 /*
  * hisi_lpc_acpi_probe - probe children for ACPI FW
  * @hostdev: LPC host device pointer
@@ -555,8 +566,7 @@ static int hisi_lpc_acpi_probe(struct device *hostdev)
 	return 0;
 
 fail:
-	device_for_each_child(hostdev, NULL,
-			      hisi_lpc_acpi_remove_subdev);
+	hisi_lpc_acpi_remove(hostdev);
 	return ret;
 }
 
@@ -626,6 +636,8 @@ static int hisi_lpc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	dev_set_drvdata(dev, lpcdev);
+
 	io_end = lpcdev->io_host->io_start + lpcdev->io_host->size;
 	dev_info(dev, "registered range [%pa - %pa]\n",
 		 &lpcdev->io_host->io_start, &io_end);
@@ -633,6 +645,23 @@ static int hisi_lpc_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static int hisi_lpc_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct acpi_device *acpi_device = ACPI_COMPANION(dev);
+	struct hisi_lpc_dev *lpcdev = dev_get_drvdata(dev);
+	struct logic_pio_hwaddr *range = lpcdev->io_host;
+
+	if (acpi_device)
+		hisi_lpc_acpi_remove(dev);
+	else
+		of_platform_depopulate(dev);
+
+	logic_pio_unregister_range(range);
+
+	return 0;
+}
+
 static const struct of_device_id hisi_lpc_of_match[] = {
 	{ .compatible = "hisilicon,hip06-lpc", },
 	{ .compatible = "hisilicon,hip07-lpc", },
@@ -646,5 +675,6 @@ static struct platform_driver hisi_lpc_driver = {
 		.acpi_match_table = ACPI_PTR(hisi_lpc_acpi_match),
 	},
 	.probe = hisi_lpc_probe,
+	.remove = hisi_lpc_remove,
 };
 builtin_platform_driver(hisi_lpc_driver);
-- 
2.17.1


^ permalink raw reply related

* [PATCH 0/5] Fixes for HiSilicon LPC driver and logical PIO code
From: John Garry @ 2019-06-20 10:31 UTC (permalink / raw)
  To: xuwei5; +Cc: bhelgaas, linuxarm, arm, linux-kernel, linux-pci, joe, John Garry

As reported in [1], the hisi-lpc driver has certain issues in handling
logical PIO regions, specifically unregistering regions.

This series add a method to unregister a logical PIO region, and fixes up
the driver to use them.

RCU usage in logical PIO code looks to always have been broken, so that
is fixed also. This is not a major fix as the list which RCU protects is
very rarely modified.

There is another patch to simplify logical PIO registration, made possible
by the fixes.

At this point, there are still separate ongoing discussions about how to
stop the logical PIO and PCI host bridge code leaking ranges, as in [2].

Hopefully this series can go through the arm soc tree and the maintainers
have no issue with that. I'm talking specifically about the logical PIO
code, which went through PCI tree on only previous upstreaming.

Cc. linux-pci@vger.kernel.org

[1] https://lore.kernel.org/lkml/1560770148-57960-1-git-send-email-john.garry@huawei.com/
[2] https://lore.kernel.org/lkml/4b24fd36-e716-7c5e-31cc-13da727802e7@huawei.com/

John Garry (5):
  lib: logic_pio: Fix RCU usage
  lib: logic_pio: Add logic_pio_unregister_range()
  bus: hisi_lpc: Unregister logical PIO range to avoid potential
    use-after-free
  bus: hisi_lpc: Add .remove method to avoid driver unbind crash
  lib: logic_pio: Enforce LOGIC_PIO_INDIRECT region ops are set at
    registration

 drivers/bus/hisi_lpc.c    | 43 ++++++++++++++++++---
 include/linux/logic_pio.h |  1 +
 lib/logic_pio.c           | 78 ++++++++++++++++++++++++++++-----------
 3 files changed, 95 insertions(+), 27 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH v2 3/4] drm/i915: Nuke drm_driver irq vfuncs
From: Ville Syrjala @ 2019-06-20 10:33 UTC (permalink / raw)
  To: intel-gfx
In-Reply-To: <20190619170842.20579-4-ville.syrjala@linux.intel.com>

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Stop using the irq vfuncs under drm_driver. That's not going to fly
in a mixed gen environment since the structure is shared between all
the devices.

v2: Allow intel_irq_uninstall() to be called twice due to
    intel_modeset_cleanup() calling it as well. Toss in a
    FIXME to remind us that this is not great.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
---
 drivers/gpu/drm/i915/i915_drv.c |   2 +-
 drivers/gpu/drm/i915/i915_irq.c | 289 ++++++++++++++++----------------
 2 files changed, 149 insertions(+), 142 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f62e3397d936..ea6b06109d5a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -754,7 +754,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 cleanup_modeset:
 	intel_modeset_cleanup(dev);
 cleanup_irq:
-	drm_irq_uninstall(dev);
+	intel_irq_uninstall(dev_priv);
 	intel_gmbus_teardown(dev_priv);
 cleanup_csr:
 	intel_csr_ucode_fini(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4fbe8d90950a..c76c498769d8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2187,8 +2187,7 @@ static void i9xx_hpd_irq_handler(struct drm_i915_private *dev_priv,
 
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -2273,8 +2272,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 
 static irqreturn_t cherryview_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -2693,8 +2691,7 @@ static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
  */
 static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	u32 de_iir, gt_iir, de_ier, sde_ier = 0;
 	irqreturn_t ret = IRQ_NONE;
 
@@ -3004,7 +3001,7 @@ static inline void gen8_master_intr_enable(void __iomem * const regs)
 
 static irqreturn_t gen8_irq_handler(int irq, void *arg)
 {
-	struct drm_i915_private *dev_priv = to_i915(arg);
+	struct drm_i915_private *dev_priv = arg;
 	void __iomem * const regs = dev_priv->uncore.regs;
 	u32 master_ctl;
 	u32 gt_iir[4];
@@ -3203,7 +3200,7 @@ static inline void gen11_master_intr_enable(void __iomem * const regs)
 
 static irqreturn_t gen11_irq_handler(int irq, void *arg)
 {
-	struct drm_i915_private * const i915 = to_i915(arg);
+	struct drm_i915_private * const i915 = arg;
 	void __iomem * const regs = i915->uncore.regs;
 	u32 master_ctl;
 	u32 gu_misc_iir;
@@ -3457,10 +3454,8 @@ static void ibx_irq_reset(struct drm_i915_private *dev_priv)
  *
  * This function needs to be called before interrupts are enabled.
  */
-static void ibx_irq_pre_postinstall(struct drm_device *dev)
+static void ibx_irq_pre_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	if (HAS_PCH_NOP(dev_priv))
 		return;
 
@@ -3529,9 +3524,8 @@ static void vlv_display_irq_postinstall(struct drm_i915_private *dev_priv)
 
 /* drm_dma.h hooks
 */
-static void ironlake_irq_reset(struct drm_device *dev)
+static void ironlake_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	GEN3_IRQ_RESET(uncore, DE);
@@ -3548,10 +3542,8 @@ static void ironlake_irq_reset(struct drm_device *dev)
 	ibx_irq_reset(dev_priv);
 }
 
-static void valleyview_irq_reset(struct drm_device *dev)
+static void valleyview_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	I915_WRITE(VLV_MASTER_IER, 0);
 	POSTING_READ(VLV_MASTER_IER);
 
@@ -3573,9 +3565,8 @@ static void gen8_gt_irq_reset(struct drm_i915_private *dev_priv)
 	GEN8_IRQ_RESET_NDX(uncore, GT, 3);
 }
 
-static void gen8_irq_reset(struct drm_device *dev)
+static void gen8_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	int pipe;
 
@@ -3618,9 +3609,8 @@ static void gen11_gt_irq_reset(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN11_GUC_SG_INTR_MASK,  ~0);
 }
 
-static void gen11_irq_reset(struct drm_device *dev)
+static void gen11_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	int pipe;
 
@@ -3693,9 +3683,8 @@ void gen8_irq_power_well_pre_disable(struct drm_i915_private *dev_priv,
 	synchronize_irq(dev_priv->drm.irq);
 }
 
-static void cherryview_irq_reset(struct drm_device *dev)
+static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	I915_WRITE(GEN8_MASTER_IRQ, 0);
@@ -3960,9 +3949,8 @@ static void bxt_hpd_irq_setup(struct drm_i915_private *dev_priv)
 	__bxt_hpd_detection_setup(dev_priv, enabled_irqs);
 }
 
-static void ibx_irq_postinstall(struct drm_device *dev)
+static void ibx_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 mask;
 
 	if (HAS_PCH_NOP(dev_priv))
@@ -3985,9 +3973,8 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 		spt_hpd_detection_setup(dev_priv);
 }
 
-static void gen5_gt_irq_postinstall(struct drm_device *dev)
+static void gen5_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 pm_irqs, gt_irqs;
 
@@ -4024,9 +4011,8 @@ static void gen5_gt_irq_postinstall(struct drm_device *dev)
 	}
 }
 
-static int ironlake_irq_postinstall(struct drm_device *dev)
+static void ironlake_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 display_mask, extra_mask;
 
@@ -4053,16 +4039,16 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 
 	dev_priv->irq_mask = ~display_mask;
 
-	ibx_irq_pre_postinstall(dev);
+	ibx_irq_pre_postinstall(dev_priv);
 
 	GEN3_IRQ_INIT(uncore, DE, dev_priv->irq_mask,
 		      display_mask | extra_mask);
 
-	gen5_gt_irq_postinstall(dev);
+	gen5_gt_irq_postinstall(dev_priv);
 
 	ilk_hpd_detection_setup(dev_priv);
 
-	ibx_irq_postinstall(dev);
+	ibx_irq_postinstall(dev_priv);
 
 	if (IS_IRONLAKE_M(dev_priv)) {
 		/* Enable PCU event interrupts
@@ -4074,8 +4060,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 		ilk_enable_display_irq(dev_priv, DE_PCU_EVENT);
 		spin_unlock_irq(&dev_priv->irq_lock);
 	}
-
-	return 0;
 }
 
 void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv)
@@ -4107,11 +4091,9 @@ void valleyview_disable_display_irqs(struct drm_i915_private *dev_priv)
 }
 
 
-static int valleyview_irq_postinstall(struct drm_device *dev)
+static void valleyview_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	gen5_gt_irq_postinstall(dev);
+	gen5_gt_irq_postinstall(dev_priv);
 
 	spin_lock_irq(&dev_priv->irq_lock);
 	if (dev_priv->display_irqs_enabled)
@@ -4120,8 +4102,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 
 	I915_WRITE(VLV_MASTER_IER, MASTER_INTERRUPT_ENABLE);
 	POSTING_READ(VLV_MASTER_IER);
-
-	return 0;
 }
 
 static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv)
@@ -4228,22 +4208,18 @@ static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv)
 	}
 }
 
-static int gen8_irq_postinstall(struct drm_device *dev)
+static void gen8_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	if (HAS_PCH_SPLIT(dev_priv))
-		ibx_irq_pre_postinstall(dev);
+		ibx_irq_pre_postinstall(dev_priv);
 
 	gen8_gt_irq_postinstall(dev_priv);
 	gen8_de_irq_postinstall(dev_priv);
 
 	if (HAS_PCH_SPLIT(dev_priv))
-		ibx_irq_postinstall(dev);
+		ibx_irq_postinstall(dev_priv);
 
 	gen8_master_intr_enable(dev_priv->uncore.regs);
-
-	return 0;
 }
 
 static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
@@ -4277,9 +4253,8 @@ static void gen11_gt_irq_postinstall(struct drm_i915_private *dev_priv)
 	I915_WRITE(GEN11_GUC_SG_INTR_MASK,  ~0);
 }
 
-static void icp_irq_postinstall(struct drm_device *dev)
+static void icp_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	u32 mask = SDE_GMBUS_ICP;
 
 	WARN_ON(I915_READ(SDEIER) != 0);
@@ -4292,14 +4267,13 @@ static void icp_irq_postinstall(struct drm_device *dev)
 	icp_hpd_detection_setup(dev_priv);
 }
 
-static int gen11_irq_postinstall(struct drm_device *dev)
+static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 gu_misc_masked = GEN11_GU_MISC_GSE;
 
 	if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
-		icp_irq_postinstall(dev);
+		icp_irq_postinstall(dev_priv);
 
 	gen11_gt_irq_postinstall(dev_priv);
 	gen8_de_irq_postinstall(dev_priv);
@@ -4310,14 +4284,10 @@ static int gen11_irq_postinstall(struct drm_device *dev)
 
 	gen11_master_intr_enable(dev_priv->uncore.regs);
 	POSTING_READ(GEN11_GFX_MSTR_IRQ);
-
-	return 0;
 }
 
-static int cherryview_irq_postinstall(struct drm_device *dev)
+static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
 	gen8_gt_irq_postinstall(dev_priv);
 
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -4327,13 +4297,10 @@ static int cherryview_irq_postinstall(struct drm_device *dev)
 
 	I915_WRITE(GEN8_MASTER_IRQ, GEN8_MASTER_IRQ_CONTROL);
 	POSTING_READ(GEN8_MASTER_IRQ);
-
-	return 0;
 }
 
-static void i8xx_irq_reset(struct drm_device *dev)
+static void i8xx_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	i9xx_pipestat_irq_reset(dev_priv);
@@ -4341,9 +4308,8 @@ static void i8xx_irq_reset(struct drm_device *dev)
 	GEN2_IRQ_RESET(uncore);
 }
 
-static int i8xx_irq_postinstall(struct drm_device *dev)
+static void i8xx_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u16 enable_mask;
 
@@ -4372,8 +4338,6 @@ static int i8xx_irq_postinstall(struct drm_device *dev)
 	i915_enable_pipestat(dev_priv, PIPE_A, PIPE_CRC_DONE_INTERRUPT_STATUS);
 	i915_enable_pipestat(dev_priv, PIPE_B, PIPE_CRC_DONE_INTERRUPT_STATUS);
 	spin_unlock_irq(&dev_priv->irq_lock);
-
-	return 0;
 }
 
 static void i8xx_error_irq_ack(struct drm_i915_private *i915,
@@ -4454,8 +4418,7 @@ static void i9xx_error_irq_handler(struct drm_i915_private *dev_priv,
 
 static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -4498,9 +4461,8 @@ static irqreturn_t i8xx_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void i915_irq_reset(struct drm_device *dev)
+static void i915_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	if (I915_HAS_HOTPLUG(dev_priv)) {
@@ -4513,9 +4475,8 @@ static void i915_irq_reset(struct drm_device *dev)
 	GEN3_IRQ_RESET(uncore, GEN2_);
 }
 
-static int i915_irq_postinstall(struct drm_device *dev)
+static void i915_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 enable_mask;
 
@@ -4553,14 +4514,11 @@ static int i915_irq_postinstall(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	i915_enable_asle_pipestat(dev_priv);
-
-	return 0;
 }
 
 static irqreturn_t i915_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -4611,9 +4569,8 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
 	return ret;
 }
 
-static void i965_irq_reset(struct drm_device *dev)
+static void i965_irq_reset(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 
 	i915_hotplug_interrupt_update(dev_priv, 0xffffffff, 0);
@@ -4624,9 +4581,8 @@ static void i965_irq_reset(struct drm_device *dev)
 	GEN3_IRQ_RESET(uncore, GEN2_);
 }
 
-static int i965_irq_postinstall(struct drm_device *dev)
+static void i965_irq_postinstall(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_uncore *uncore = &dev_priv->uncore;
 	u32 enable_mask;
 	u32 error_mask;
@@ -4676,8 +4632,6 @@ static int i965_irq_postinstall(struct drm_device *dev)
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	i915_enable_asle_pipestat(dev_priv);
-
-	return 0;
 }
 
 static void i915_hpd_irq_setup(struct drm_i915_private *dev_priv)
@@ -4707,8 +4661,7 @@ static void i915_hpd_irq_setup(struct drm_i915_private *dev_priv)
 
 static irqreturn_t i965_irq_handler(int irq, void *arg)
 {
-	struct drm_device *dev = arg;
-	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_i915_private *dev_priv = arg;
 	irqreturn_t ret = IRQ_NONE;
 
 	if (!intel_irqs_enabled(dev_priv))
@@ -4839,65 +4792,18 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 	dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
 	dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
 
-	if (IS_CHERRYVIEW(dev_priv)) {
-		dev->driver->irq_handler = cherryview_irq_handler;
-		dev->driver->irq_preinstall = cherryview_irq_reset;
-		dev->driver->irq_postinstall = cherryview_irq_postinstall;
-		dev->driver->irq_uninstall = cherryview_irq_reset;
-		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
-	} else if (IS_VALLEYVIEW(dev_priv)) {
-		dev->driver->irq_handler = valleyview_irq_handler;
-		dev->driver->irq_preinstall = valleyview_irq_reset;
-		dev->driver->irq_postinstall = valleyview_irq_postinstall;
-		dev->driver->irq_uninstall = valleyview_irq_reset;
-		dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
-	} else if (INTEL_GEN(dev_priv) >= 11) {
-		dev->driver->irq_handler = gen11_irq_handler;
-		dev->driver->irq_preinstall = gen11_irq_reset;
-		dev->driver->irq_postinstall = gen11_irq_postinstall;
-		dev->driver->irq_uninstall = gen11_irq_reset;
-		dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;
-	} else if (INTEL_GEN(dev_priv) >= 8) {
-		dev->driver->irq_handler = gen8_irq_handler;
-		dev->driver->irq_preinstall = gen8_irq_reset;
-		dev->driver->irq_postinstall = gen8_irq_postinstall;
-		dev->driver->irq_uninstall = gen8_irq_reset;
-		if (IS_GEN9_LP(dev_priv))
+	if (HAS_GMCH(dev_priv)) {
+		if (I915_HAS_HOTPLUG(dev_priv))
+			dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			dev_priv->display.hpd_irq_setup = gen11_hpd_irq_setup;
+		else if (IS_GEN9_LP(dev_priv))
 			dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
 		else if (INTEL_PCH_TYPE(dev_priv) >= PCH_SPT)
 			dev_priv->display.hpd_irq_setup = spt_hpd_irq_setup;
 		else
 			dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
-	} else if (HAS_PCH_SPLIT(dev_priv)) {
-		dev->driver->irq_handler = ironlake_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_reset;
-		dev->driver->irq_postinstall = ironlake_irq_postinstall;
-		dev->driver->irq_uninstall = ironlake_irq_reset;
-		dev_priv->display.hpd_irq_setup = ilk_hpd_irq_setup;
-	} else {
-		if (IS_GEN(dev_priv, 2)) {
-			dev->driver->irq_preinstall = i8xx_irq_reset;
-			dev->driver->irq_postinstall = i8xx_irq_postinstall;
-			dev->driver->irq_handler = i8xx_irq_handler;
-			dev->driver->irq_uninstall = i8xx_irq_reset;
-		} else if (IS_I945GM(dev_priv)) {
-			dev->driver->irq_preinstall = i915_irq_reset;
-			dev->driver->irq_postinstall = i915_irq_postinstall;
-			dev->driver->irq_uninstall = i915_irq_reset;
-			dev->driver->irq_handler = i915_irq_handler;
-		} else if (IS_GEN(dev_priv, 3)) {
-			dev->driver->irq_preinstall = i915_irq_reset;
-			dev->driver->irq_postinstall = i915_irq_postinstall;
-			dev->driver->irq_uninstall = i915_irq_reset;
-			dev->driver->irq_handler = i915_irq_handler;
-		} else {
-			dev->driver->irq_preinstall = i965_irq_reset;
-			dev->driver->irq_postinstall = i965_irq_postinstall;
-			dev->driver->irq_uninstall = i965_irq_reset;
-			dev->driver->irq_handler = i965_irq_handler;
-		}
-		if (I915_HAS_HOTPLUG(dev_priv))
-			dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
 	}
 }
 
@@ -4918,6 +4824,75 @@ void intel_irq_fini(struct drm_i915_private *i915)
 		kfree(i915->l3_parity.remap_info[i]);
 }
 
+static irq_handler_t intel_irq_handler(struct drm_i915_private *dev_priv)
+{
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			return cherryview_irq_handler;
+		else if (IS_VALLEYVIEW(dev_priv))
+			return valleyview_irq_handler;
+		else if (IS_GEN(dev_priv, 4))
+			return i965_irq_handler;
+		else if (IS_GEN(dev_priv, 3))
+			return i915_irq_handler;
+		else
+			return i8xx_irq_handler;
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			return gen11_irq_handler;
+		else if (INTEL_GEN(dev_priv) >= 8)
+			return gen8_irq_handler;
+		else
+			return ironlake_irq_handler;
+	}
+}
+
+static void intel_irq_reset(struct drm_i915_private *dev_priv)
+{
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			cherryview_irq_reset(dev_priv);
+		else if (IS_VALLEYVIEW(dev_priv))
+			valleyview_irq_reset(dev_priv);
+		else if (IS_GEN(dev_priv, 4))
+			i965_irq_reset(dev_priv);
+		else if (IS_GEN(dev_priv, 3))
+			i915_irq_reset(dev_priv);
+		else
+			i8xx_irq_reset(dev_priv);
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			gen11_irq_reset(dev_priv);
+		else if (INTEL_GEN(dev_priv) >= 8)
+			gen8_irq_reset(dev_priv);
+		else
+			ironlake_irq_reset(dev_priv);
+	}
+}
+
+static void intel_irq_postinstall(struct drm_i915_private *dev_priv)
+{
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			cherryview_irq_postinstall(dev_priv);
+		else if (IS_VALLEYVIEW(dev_priv))
+			valleyview_irq_postinstall(dev_priv);
+		else if (IS_GEN(dev_priv, 4))
+			i965_irq_postinstall(dev_priv);
+		else if (IS_GEN(dev_priv, 3))
+			i915_irq_postinstall(dev_priv);
+		else
+			i8xx_irq_postinstall(dev_priv);
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			gen11_irq_postinstall(dev_priv);
+		else if (INTEL_GEN(dev_priv) >= 8)
+			gen8_irq_postinstall(dev_priv);
+		else
+			ironlake_irq_postinstall(dev_priv);
+	}
+}
+
 /**
  * intel_irq_install - enables the hardware interrupt
  * @dev_priv: i915 device instance
@@ -4931,6 +4906,9 @@ void intel_irq_fini(struct drm_i915_private *i915)
  */
 int intel_irq_install(struct drm_i915_private *dev_priv)
 {
+	int irq = dev_priv->drm.pdev->irq;
+	int ret;
+
 	/*
 	 * We enable some interrupt sources in our postinstall hooks, so mark
 	 * interrupts as enabled _before_ actually enabling them to avoid
@@ -4938,7 +4916,20 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
 	 */
 	dev_priv->runtime_pm.irqs_enabled = true;
 
-	return drm_irq_install(&dev_priv->drm, dev_priv->drm.pdev->irq);
+	dev_priv->drm.irq_enabled = true;
+
+	intel_irq_reset(dev_priv);
+
+	ret = request_irq(irq, intel_irq_handler(dev_priv),
+			  IRQF_SHARED, DRIVER_NAME, dev_priv);
+	if (ret < 0) {
+		dev_priv->drm.irq_enabled = false;
+		return ret;
+	}
+
+	intel_irq_postinstall(dev_priv);
+
+	return ret;
 }
 
 /**
@@ -4950,7 +4941,23 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
  */
 void intel_irq_uninstall(struct drm_i915_private *dev_priv)
 {
-	drm_irq_uninstall(&dev_priv->drm);
+	int irq = dev_priv->drm.pdev->irq;
+
+	/*
+	 * FIXME we can get called twice during driver load
+	 * error handling due to intel_modeset_cleanup()
+	 * calling us out of sequence. Would be nice if
+	 * it didn't do that...
+	 */
+	if (!dev_priv->drm.irq_enabled)
+		return;
+
+	dev_priv->drm.irq_enabled = false;
+
+	intel_irq_reset(dev_priv);
+
+	free_irq(irq, dev_priv);
+
 	intel_hpd_cancel_work(dev_priv);
 	dev_priv->runtime_pm.irqs_enabled = false;
 }
@@ -4964,7 +4971,7 @@ void intel_irq_uninstall(struct drm_i915_private *dev_priv)
  */
 void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
 {
-	dev_priv->drm.driver->irq_uninstall(&dev_priv->drm);
+	intel_irq_reset(dev_priv);
 	dev_priv->runtime_pm.irqs_enabled = false;
 	synchronize_irq(dev_priv->drm.irq);
 }
@@ -4979,6 +4986,6 @@ void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv)
 void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv)
 {
 	dev_priv->runtime_pm.irqs_enabled = true;
-	dev_priv->drm.driver->irq_preinstall(&dev_priv->drm);
-	dev_priv->drm.driver->irq_postinstall(&dev_priv->drm);
+	intel_irq_reset(dev_priv);
+	intel_irq_postinstall(dev_priv);
 }
-- 
2.21.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related

* [PATCH 2/5] lib: logic_pio: Add logic_pio_unregister_range()
From: John Garry @ 2019-06-20 10:31 UTC (permalink / raw)
  To: xuwei5; +Cc: bhelgaas, linuxarm, arm, linux-kernel, linux-pci, joe, John Garry
In-Reply-To: <1561026716-140537-1-git-send-email-john.garry@huawei.com>

Add a function to unregister a logical PIO range.

The method used to allocate LOGIC_PIO_CPU_MMIO regions during registration
is slightly modified to ensure that we get no overlap when regions are
unregistered. This is needed because the allocation scheme assumed that no
regions are ever unregistered.

Logical PIO space can still be leaked when unregistering certain
LOGIC_PIO_CPU_MMIO regions, but this acceptable for now since there are no
callers to unregister LOGIC_PIO_CPU_MMIO regions, and the logical PIO
region allocation scheme would need significant work to improve this.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 include/linux/logic_pio.h |  1 +
 lib/logic_pio.c           | 16 +++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/logic_pio.h b/include/linux/logic_pio.h
index cbd9d8495690..88e1e6304a71 100644
--- a/include/linux/logic_pio.h
+++ b/include/linux/logic_pio.h
@@ -117,6 +117,7 @@ struct logic_pio_hwaddr *find_io_range_by_fwnode(struct fwnode_handle *fwnode);
 unsigned long logic_pio_trans_hwaddr(struct fwnode_handle *fwnode,
 			resource_size_t hw_addr, resource_size_t size);
 int logic_pio_register_range(struct logic_pio_hwaddr *newrange);
+void logic_pio_unregister_range(struct logic_pio_hwaddr *range);
 resource_size_t logic_pio_to_hwaddr(unsigned long pio);
 unsigned long logic_pio_trans_cpuaddr(resource_size_t hw_addr);
 
diff --git a/lib/logic_pio.c b/lib/logic_pio.c
index 761296376fbc..45eb57af2574 100644
--- a/lib/logic_pio.c
+++ b/lib/logic_pio.c
@@ -56,7 +56,7 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
 			/* for MMIO ranges we need to check for overlap */
 			if (start >= range->hw_start + range->size ||
 			    end < range->hw_start) {
-				mmio_sz += range->size;
+				mmio_sz = range->io_start + range->size;
 			} else {
 				ret = -EFAULT;
 				goto end_register;
@@ -98,6 +98,20 @@ int logic_pio_register_range(struct logic_pio_hwaddr *new_range)
 	return ret;
 }
 
+/**
+ * logic_pio_unregister_range - unregister logical PIO range for a host
+ * @range: pointer to the IO range which has been already registered.
+ *
+ * Unregister a previously-registered IO range node.
+ */
+void logic_pio_unregister_range(struct logic_pio_hwaddr *range)
+{
+	mutex_lock(&io_range_mutex);
+	list_del_rcu(&range->list);
+	mutex_unlock(&io_range_mutex);
+	synchronize_rcu();
+}
+
 /**
  * find_io_range_by_fwnode - find logical PIO range for given FW node
  * @fwnode: FW node handle associated with logical PIO range
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 4/5] mm: introduce MADV_PAGEOUT
From: Minchan Kim @ 2019-06-20 10:32 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, LKML, linux-api, Johannes Weiner,
	Tim Murray, Joel Fernandes, Suren Baghdasaryan, Daniel Colascione,
	Shakeel Butt, Sonny Rao, Brian Geffon, jannh, oleg, christian,
	oleksandr, hdanton, lizeb
In-Reply-To: <20190620092209.GD12083@dhcp22.suse.cz>

On Thu, Jun 20, 2019 at 11:22:09AM +0200, Michal Hocko wrote:
> On Thu 20-06-19 17:40:40, Minchan Kim wrote:
> > > > > Pushing out a shared page cache
> > > > > is possible even now but this interface gives a much easier tool to
> > > > > evict shared state and perform all sorts of timing attacks. Unless I am
> > > > > missing something we should be doing something similar to mincore and
> > > > > ignore shared pages without a writeable access or at least document why
> > > > > we do not care.
> > > > 
> > > > I'm not sure IIUC side channel attach. As you mentioned, without this syscall,
> > > > 1. they already can do that simply by memory hogging
> > > 
> > > This is way much more harder for practical attacks because the reclaim
> > > logic is not fully under the attackers control. Having a direct tool to
> > > reclaim memory directly then just opens doors to measure the other
> > > consumers of that memory and all sorts of side channel.
> > 
> > Not sure it's much more harder. It's really easy on my experience.
> > Just creating new memory hogger and consume memory step by step until
> > you newly allocated pages will be reclaimed.
> 
> You can contain an untrusted application into a memcg and it will only
> reclaim its own working set.
> 
> > > > 2. If we need fix MADV_PAGEOUT, that means we need to fix MADV_DONTNEED, too?
> > > 
> > > nope because MADV_DONTNEED doesn't unmap from other processes.
> > 
> > Hmm, I don't understand. MADV_PAGEOUT doesn't unmap from other
> > processes, either.
> 
> Either I am confused or missing something. shrink_page_list does
> try_to_unmap and that unmaps from all processes, right?

You don't miss it. It seems now I undetstand what you pointed out.
What you meant is attacker can see what page was faulting-in from other processes
via measuring access delay from his address space and MADV_PAGEOUT makes it more
easiler. Thus, it's an issue regardless of recent mincore fix. Right?
Then, okay, I will add can_do_mincore similar check for the MADV_PAGEOUT syscall
if others have different ideas.

Thanks.

^ permalink raw reply

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/selftests: Use request managed wakerefs
From: Patchwork @ 2019-06-20 10:31 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx
In-Reply-To: <20190620102432.31580-1-chris@chris-wilson.co.uk>

== Series Details ==

Series: drm/i915/selftests: Use request managed wakerefs
URL   : https://patchwork.freedesktop.org/series/62445/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
2c57955587cb drm/i915/selftests: Use request managed wakerefs
-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")'
#11: 
References: 79ffac8599c4 ("drm/i915: Invert the GEM wakeref hierarchy")

total: 1 errors, 0 warnings, 0 checks, 140 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* [U-Boot] [PATCH v5 6/8] net: macb: Extend MACB driver for SiFive Unleashed board
From: Ramon Fried @ 2019-06-20 10:31 UTC (permalink / raw)
  To: u-boot
In-Reply-To: <MN2PR04MB6061DAA6C82AD2846AB76D688DE40@MN2PR04MB6061.namprd04.prod.outlook.com>


On 6/20/19 12:55 PM, Anup Patel wrote:
>
>> -----Original Message-----
>> From: Ramon Fried <rfried.dev@gmail.com>
>> Sent: Thursday, June 20, 2019 2:48 PM
>> To: Anup Patel <Anup.Patel@wdc.com>
>> Cc: Rick Chen <rick@andestech.com>; Bin Meng <bmeng.cn@gmail.com>;
>> Lukas Auer <lukas.auer@aisec.fraunhofer.de>; Simon Glass
>> <sjg@chromium.org>; Joe Hershberger <joe.hershberger@ni.com>; Palmer
>> Dabbelt <palmer@sifive.com>; Paul Walmsley <paul.walmsley@sifive.com>;
>> Troy Benjegerdes <troy.benjegerdes@sifive.com>; Atish Patra
>> <Atish.Patra@wdc.com>; Alistair Francis <Alistair.Francis@wdc.com>; U-Boot
>> Mailing List <u-boot@lists.denx.de>
>> Subject: Re: [PATCH v5 6/8] net: macb: Extend MACB driver for SiFive
>> Unleashed board
>>
>>
>> On Thu, Jun 20, 2019 at 9:49 AM Anup Patel <Anup.Patel@wdc.com> wrote:
>>> The SiFive MACB ethernet has a custom TX_CLK_SEL register to select
>>> different TX clock for 1000mbps vs 10/100mbps.
>>>
>>> This patch adds SiFive MACB compatible string and extends the MACB
>>> ethernet driver to change TX clock using TX_CLK_SEL register for
>>> SiFive MACB.
>>>
>>> Signed-off-by: Anup Patel <anup.patel@wdc.com>
>>> ---
>>>  drivers/net/macb.c | 53
>>> +++++++++++++++++++++++++++++++++++++++-------
>>>  1 file changed, 45 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c index
>>> c072f99d8f..6a29ee3064 100644
>>> --- a/drivers/net/macb.c
>>> +++ b/drivers/net/macb.c
>>> @@ -83,6 +83,9 @@ struct macb_dma_desc {
>>>
>>>  struct macb_device {
>>>         void                    *regs;
>>> +       void                    *regs_sifive_gemgxl;
>> This needs needs to be part of the specific config structure.
> Sure, these are SOC specific so let me place it in
> config structure.
>
>>> +
>>> +       bool                    skip_dma_config;
>> Why do you ever need to skip_dma_config ?
> I tried all possible dma_burst_length from 0 to 16 but
> none of them worked.
>
> At the moment, with gmac_configure_dma() in-place
> this driver has stopped working on SiFive Unleashed board.
>
> Instead of skip_dma_config, I think dma_burst_length ==0 
> should be treated as skip gmac_configure_dma().

This is interesting, can you please print the dmacfg register before and after gma_configure_dma() ?

I tested this on two different boards, but maybe I missed something.


>
>>>         unsigned int            dma_burst_length;
> I was confused here. You have dma_burst_length here
> and in config structure as well. Why ?

I only had one configuration property, so I just copied it, but now that you've added more configuration items, it's reasonable to put a pointer to the config structure and use the dma_burst_length directly from there.


>
>>>         unsigned int            rx_tail;
>>> @@ -122,7 +125,9 @@ struct macb_device {  };
>>>
>>>  struct macb_config {
>>> +       bool                    skip_dma_config;
>>>         unsigned int            dma_burst_length;
>>> +       bool                    has_sifive_gemgxl;
>>>  };
>>>
>>>  #ifndef CONFIG_DM_ETH
>>> @@ -486,18 +491,11 @@ static int macb_phy_find(struct macb_device
>>> *macb, const char *name)  int __weak macb_linkspd_cb(struct udevice
>>> *dev, unsigned int speed)  {  #ifdef CONFIG_CLK
>>> +       struct macb_device *macb = dev_get_priv(dev);
>>>         struct clk tx_clk;
>>>         ulong rate;
>>>         int ret;
>>>
>>> -       /*
>>> -        * "tx_clk" is an optional clock source for MACB.
>>> -        * Ignore if it does not exist in DT.
>>> -        */
>>> -       ret = clk_get_by_name(dev, "tx_clk", &tx_clk);
>>> -       if (ret)
>>> -               return 0;
>>> -
>>>         switch (speed) {
>>>         case _10BASET:
>>>                 rate = 2500000;         /* 2.5 MHz */
>>> @@ -513,6 +511,26 @@ int __weak macb_linkspd_cb(struct udevice *dev,
>> unsigned int speed)
>>>                 return 0;
>>>         }
>>>
>>> +       if (macb->regs_sifive_gemgxl) {
>>> +               /*
>>> +                * SiFive GEMGXL TX clock operation mode:
>>> +                *
>>> +                * 0 = GMII mode. Use 125 MHz gemgxlclk from PRCI in TX logic
>>> +                *     and output clock on GMII output signal GTX_CLK
>>> +                * 1 = MII mode. Use MII input signal TX_CLK in TX logic
>>> +                */
>>> +               writel(rate != 125000000, macb->regs_sifive_gemgxl);
>>> +               return 0;
>>> +       }
>> move to dedicated clock init.
> sure, I will add clock_init() callback in macb_config
>
>>> +
>>> +       /*
>>> +        * "tx_clk" is an optional clock source for MACB.
>>> +        * Ignore if it does not exist in DT.
>>> +        */
>>> +       ret = clk_get_by_name(dev, "tx_clk", &tx_clk);
>>> +       if (ret)
>>> +               return 0;
>>> +
>>>         if (tx_clk.dev) {
>>>                 ret = clk_set_rate(&tx_clk, rate);
>>>                 if (ret)
>>> @@ -701,6 +719,9 @@ static void gmac_configure_dma(struct
>> macb_device *macb)
>>>         u32 buffer_size;
>>>         u32 dmacfg;
>>>
>>> +       if (macb->skip_dma_config)
>>> +               return;
>>> +
>> Same as before, why do you skip it ?
> Same as above comment.
>
>>>         buffer_size = 128 / RX_BUFFER_MULTIPLE;
>>>         dmacfg = gem_readl(macb, DMACFG) & ~GEM_BF(RXBS, -1L);
>>>         dmacfg |= GEM_BF(RXBS, buffer_size); @@ -1178,6 +1199,7 @@
>>> static int macb_eth_probe(struct udevice *dev)
>>>         struct macb_device *macb = dev_get_priv(dev);
>>>         const char *phy_mode;
>>>         __maybe_unused int ret;
>>> +       fdt_addr_t addr;
>>>
>>>         phy_mode = fdt_getprop(gd->fdt_blob, dev_of_offset(dev), "phy-
>> mode",
>>>                                NULL);
>>> @@ -1194,6 +1216,7 @@ static int macb_eth_probe(struct udevice *dev)
>>>         if (!macb_config)
>>>                 macb_config = &default_gem_config;
>>>
>>> +       macb->skip_dma_config = macb_config->skip_dma_config;
>>>         macb->dma_burst_length = macb_config->dma_burst_length;
>>> #ifdef CONFIG_CLK
>>>         ret = macb_enable_clk(dev);
>>> @@ -1201,6 +1224,13 @@ static int macb_eth_probe(struct udevice *dev)
>>>                 return ret;
>>>  #endif
>>>
>>> +       if (macb_config->has_sifive_gemgxl) {
>>> +               addr = dev_read_addr_index(dev, 1);
>>> +               if (addr == FDT_ADDR_T_NONE)
>>> +                       return -ENODEV;
>>> +               macb->regs_sifive_gemgxl = (void __iomem *)addr;
>>> +       }
>> This should be all done in a a specific sifive init function.
>> You can define init function and clock init function CB functions in config (Like
>> in Linux):
>> "
>> int (*clk_init)(struct platform_device *pdev, struct clk **pclk, struct clk
>> **hclk, struct clk **tx_clk, struct clk **rx_clk); int (*init)(struct
>> platform_device *pdev); "
>>
>> and add your specific SOC initialization there.
>> This function should be generic as possible.
> Sure, I will add init() callback in config structure.
>
>>> +
>>>         _macb_eth_initialize(macb);
>>>
>>>  #if defined(CONFIG_CMD_MII) || defined(CONFIG_PHYLIB) @@ -1259,6
>>> +1289,12 @@ static const struct macb_config sama5d4_config = {
>>>         .dma_burst_length = 4,
>>>  };
>>>
>>> +static const struct macb_config sifive_config = {
>>> +       .skip_dma_config = true,
>>> +       .dma_burst_length = 0,
>> Don't mention it if it's zero, and by the way, I assume it should be not zero.
>> You're wasting memory cycles, try to find the correct value. (I think
>> 4 is the default in macb)
> Well, SiFive FU540 is a SiFive SOC. I don't know how they
> have integrated MACB. May be DMA configuration for
> SiFive MACB is little different.
>
> In any case, it was working before so we should atleast
> get it back to working state by skipping DMA configuration
> when dma_burst_lenght == 0.

Like I written before, additionally, I'm writing a new patch that increases buffer size to 2K for
GEM supported devices, this will go in dma configuration, it's pitty that you will miss this improvement.
Let's try to fix gmac_configure_dma() for you.

>>> +       .has_sifive_gemgxl = true,
>> can be dropped if callback functions are declared.
> Sure.
>
>>> +};
>>> +
>>>  static const struct udevice_id macb_eth_ids[] = {
>>>         { .compatible = "cdns,macb" },
>>>         { .compatible = "cdns,at91sam9260-macb" }, @@ -1266,6 +1302,7
>>> @@ static const struct udevice_id macb_eth_ids[] = {
>>>         { .compatible = "atmel,sama5d3-gem" },
>>>         { .compatible = "atmel,sama5d4-gem", .data =
>> (ulong)&sama5d4_config },
>>>         { .compatible = "cdns,zynq-gem" },
>>> +       { .compatible = "sifive,fu540-macb", .data =
>>> + (ulong)&sifive_config },
>>>         { }
>>>  };
>>>
>>> --
>>> 2.17.1
>>>
>> Thanks,
>> Ramon.
> Regards,
> Anup

^ permalink raw reply

* Re: [PATCH v10 08/13] mm/sparsemem: Prepare for sub-section ranges
From: David Hildenbrand @ 2019-06-20 10:31 UTC (permalink / raw)
  To: Dan Williams, akpm
  Cc: Michal Hocko, Vlastimil Babka, Logan Gunthorpe, Oscar Salvador,
	Pavel Tatashin, linux-mm, linux-nvdimm, linux-kernel
In-Reply-To: <156092353780.979959.9713046515562743194.stgit@dwillia2-desk3.amr.corp.intel.com>

On 19.06.19 07:52, Dan Williams wrote:
> Prepare the memory hot-{add,remove} paths for handling sub-section
> ranges by plumbing the starting page frame and number of pages being
> handled through arch_{add,remove}_memory() to
> sparse_{add,remove}_one_section().
> 
> This is simply plumbing, small cleanups, and some identifier renames. No
> intended functional changes.
> 
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Reviewed-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  include/linux/memory_hotplug.h |    5 +-
>  mm/memory_hotplug.c            |  114 +++++++++++++++++++++++++---------------
>  mm/sparse.c                    |   16 ++----
>  3 files changed, 81 insertions(+), 54 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 79e0add6a597..3ab0282b4fe5 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -348,9 +348,10 @@ extern int add_memory_resource(int nid, struct resource *resource);
>  extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  		unsigned long nr_pages, struct vmem_altmap *altmap);
>  extern bool is_memblock_offlined(struct memory_block *mem);
> -extern int sparse_add_one_section(int nid, unsigned long start_pfn,
> -				  struct vmem_altmap *altmap);
> +extern int sparse_add_section(int nid, unsigned long pfn,
> +		unsigned long nr_pages, struct vmem_altmap *altmap);
>  extern void sparse_remove_one_section(struct mem_section *ms,
> +		unsigned long pfn, unsigned long nr_pages,
>  		unsigned long map_offset, struct vmem_altmap *altmap);
>  extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
>  					  unsigned long pnum);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 4b882c57781a..399bf78bccc5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -252,51 +252,84 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
>  }
>  #endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
>  
> -static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
> -				   struct vmem_altmap *altmap)
> +static int __meminit __add_section(int nid, unsigned long pfn,
> +		unsigned long nr_pages,	struct vmem_altmap *altmap)
>  {
>  	int ret;
>  
> -	if (pfn_valid(phys_start_pfn))
> +	if (pfn_valid(pfn))
>  		return -EEXIST;
>  
> -	ret = sparse_add_one_section(nid, phys_start_pfn, altmap);
> +	ret = sparse_add_section(nid, pfn, nr_pages, altmap);
>  	return ret < 0 ? ret : 0;
>  }
>  
> +static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
> +		const char *reason)
> +{
> +	/*
> +	 * Disallow all operations smaller than a sub-section and only
> +	 * allow operations smaller than a section for
> +	 * SPARSEMEM_VMEMMAP. Note that check_hotplug_memory_range()
> +	 * enforces a larger memory_block_size_bytes() granularity for
> +	 * memory that will be marked online, so this check should only
> +	 * fire for direct arch_{add,remove}_memory() users outside of
> +	 * add_memory_resource().
> +	 */
> +	unsigned long min_align;
> +
> +	if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
> +		min_align = PAGES_PER_SUBSECTION;
> +	else
> +		min_align = PAGES_PER_SECTION;
> +	if (!IS_ALIGNED(pfn, min_align)
> +			|| !IS_ALIGNED(nr_pages, min_align)) {
> +		WARN(1, "Misaligned __%s_pages start: %#lx end: #%lx\n",
> +				reason, pfn, pfn + nr_pages - 1);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Reasonably generic function for adding memory.  It is
>   * expected that archs that support memory hotplug will
>   * call this function after deciding the zone to which to
>   * add the new pages.
>   */
> -int __ref __add_pages(int nid, unsigned long phys_start_pfn,
> -		unsigned long nr_pages, struct mhp_restrictions *restrictions)
> +int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> +		struct mhp_restrictions *restrictions)
>  {
>  	unsigned long i;
> -	int err = 0;
> -	int start_sec, end_sec;
> +	int start_sec, end_sec, err;
>  	struct vmem_altmap *altmap = restrictions->altmap;
>  
> -	/* during initialize mem_map, align hot-added range to section */
> -	start_sec = pfn_to_section_nr(phys_start_pfn);
> -	end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
> -
>  	if (altmap) {
>  		/*
>  		 * Validate altmap is within bounds of the total request
>  		 */
> -		if (altmap->base_pfn != phys_start_pfn
> +		if (altmap->base_pfn != pfn
>  				|| vmem_altmap_offset(altmap) > nr_pages) {
>  			pr_warn_once("memory add fail, invalid altmap\n");
> -			err = -EINVAL;
> -			goto out;
> +			return -EINVAL;
>  		}
>  		altmap->alloc = 0;
>  	}
>  
> +	err = check_pfn_span(pfn, nr_pages, "add");
> +	if (err)
> +		return err;
> +
> +	start_sec = pfn_to_section_nr(pfn);
> +	end_sec = pfn_to_section_nr(pfn + nr_pages - 1);
>  	for (i = start_sec; i <= end_sec; i++) {
> -		err = __add_section(nid, section_nr_to_pfn(i), altmap);
> +		unsigned long pfns;
> +
> +		pfns = min(nr_pages, PAGES_PER_SECTION
> +				- (pfn & ~PAGE_SECTION_MASK));
> +		err = __add_section(nid, pfn, pfns, altmap);
> +		pfn += pfns;
> +		nr_pages -= pfns;
>  
>  		/*
>  		 * EEXIST is finally dealt with by ioresource collision
> @@ -309,7 +342,6 @@ int __ref __add_pages(int nid, unsigned long phys_start_pfn,
>  		cond_resched();
>  	}
>  	vmemmap_populate_print_last();
> -out:
>  	return err;
>  }
>  
> @@ -487,10 +519,10 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
>  	pgdat->node_spanned_pages = 0;
>  }
>  
> -static void __remove_zone(struct zone *zone, unsigned long start_pfn)
> +static void __remove_zone(struct zone *zone, unsigned long start_pfn,
> +		unsigned long nr_pages)
>  {
>  	struct pglist_data *pgdat = zone->zone_pgdat;
> -	int nr_pages = PAGES_PER_SECTION;
>  	unsigned long flags;
>  
>  	pgdat_resize_lock(zone->zone_pgdat, &flags);
> @@ -499,27 +531,23 @@ static void __remove_zone(struct zone *zone, unsigned long start_pfn)
>  	pgdat_resize_unlock(zone->zone_pgdat, &flags);
>  }
>  
> -static void __remove_section(struct zone *zone, struct mem_section *ms,
> -			     unsigned long map_offset,
> -			     struct vmem_altmap *altmap)
> +static void __remove_section(struct zone *zone, unsigned long pfn,
> +		unsigned long nr_pages, unsigned long map_offset,
> +		struct vmem_altmap *altmap)
>  {
> -	unsigned long start_pfn;
> -	int scn_nr;
> +	struct mem_section *ms = __nr_to_section(pfn_to_section_nr(pfn));
>  
>  	if (WARN_ON_ONCE(!valid_section(ms)))
>  		return;
>  
> -	scn_nr = __section_nr(ms);
> -	start_pfn = section_nr_to_pfn((unsigned long)scn_nr);
> -	__remove_zone(zone, start_pfn);
> -
> -	sparse_remove_one_section(ms, map_offset, altmap);
> +	__remove_zone(zone, pfn, nr_pages);
> +	sparse_remove_one_section(ms, pfn, nr_pages, map_offset, altmap);
>  }
>  
>  /**
>   * __remove_pages() - remove sections of pages from a zone
>   * @zone: zone from which pages need to be removed
> - * @phys_start_pfn: starting pageframe (must be aligned to start of a section)
> + * @pfn: starting pageframe (must be aligned to start of a section)
>   * @nr_pages: number of pages to remove (must be multiple of section size)
>   * @altmap: alternative device page map or %NULL if default memmap is used
>   *
> @@ -528,31 +556,31 @@ static void __remove_section(struct zone *zone, struct mem_section *ms,
>   * sure that pages are marked reserved and zones are adjust properly by
>   * calling offline_pages().
>   */
> -void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> +void __remove_pages(struct zone *zone, unsigned long pfn,
>  		    unsigned long nr_pages, struct vmem_altmap *altmap)
>  {
> -	unsigned long i;
>  	unsigned long map_offset = 0;
> -	int sections_to_remove;
> +	int i, start_sec, end_sec;

As mentioned in v9, use "unsigned long" for start_sec and end_sec please.

-- 

Thanks,

David / dhildenb


^ permalink raw reply

* Re: [PATCH v10 08/13] mm/sparsemem: Prepare for sub-section ranges
From: David Hildenbrand @ 2019-06-20 10:31 UTC (permalink / raw)
  To: Dan Williams, akpm
  Cc: Michal Hocko, Pavel Tatashin, linux-nvdimm, linux-kernel,
	linux-mm, Vlastimil Babka, Oscar Salvador
In-Reply-To: <156092353780.979959.9713046515562743194.stgit@dwillia2-desk3.amr.corp.intel.com>

On 19.06.19 07:52, Dan Williams wrote:
> Prepare the memory hot-{add,remove} paths for handling sub-section
> ranges by plumbing the starting page frame and number of pages being
> handled through arch_{add,remove}_memory() to
> sparse_{add,remove}_one_section().
> 
> This is simply plumbing, small cleanups, and some identifier renames. No
> intended functional changes.
> 
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Oscar Salvador <osalvador@suse.de>
> Reviewed-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  include/linux/memory_hotplug.h |    5 +-
>  mm/memory_hotplug.c            |  114 +++++++++++++++++++++++++---------------
>  mm/sparse.c                    |   16 ++----
>  3 files changed, 81 insertions(+), 54 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 79e0add6a597..3ab0282b4fe5 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -348,9 +348,10 @@ extern int add_memory_resource(int nid, struct resource *resource);
>  extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  		unsigned long nr_pages, struct vmem_altmap *altmap);
>  extern bool is_memblock_offlined(struct memory_block *mem);
> -extern int sparse_add_one_section(int nid, unsigned long start_pfn,
> -				  struct vmem_altmap *altmap);
> +extern int sparse_add_section(int nid, unsigned long pfn,
> +		unsigned long nr_pages, struct vmem_altmap *altmap);
>  extern void sparse_remove_one_section(struct mem_section *ms,
> +		unsigned long pfn, unsigned long nr_pages,
>  		unsigned long map_offset, struct vmem_altmap *altmap);
>  extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
>  					  unsigned long pnum);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 4b882c57781a..399bf78bccc5 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -252,51 +252,84 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
>  }
>  #endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
>  
> -static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
> -				   struct vmem_altmap *altmap)
> +static int __meminit __add_section(int nid, unsigned long pfn,
> +		unsigned long nr_pages,	struct vmem_altmap *altmap)
>  {
>  	int ret;
>  
> -	if (pfn_valid(phys_start_pfn))
> +	if (pfn_valid(pfn))
>  		return -EEXIST;
>  
> -	ret = sparse_add_one_section(nid, phys_start_pfn, altmap);
> +	ret = sparse_add_section(nid, pfn, nr_pages, altmap);
>  	return ret < 0 ? ret : 0;
>  }
>  
> +static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
> +		const char *reason)
> +{
> +	/*
> +	 * Disallow all operations smaller than a sub-section and only
> +	 * allow operations smaller than a section for
> +	 * SPARSEMEM_VMEMMAP. Note that check_hotplug_memory_range()
> +	 * enforces a larger memory_block_size_bytes() granularity for
> +	 * memory that will be marked online, so this check should only
> +	 * fire for direct arch_{add,remove}_memory() users outside of
> +	 * add_memory_resource().
> +	 */
> +	unsigned long min_align;
> +
> +	if (IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
> +		min_align = PAGES_PER_SUBSECTION;
> +	else
> +		min_align = PAGES_PER_SECTION;
> +	if (!IS_ALIGNED(pfn, min_align)
> +			|| !IS_ALIGNED(nr_pages, min_align)) {
> +		WARN(1, "Misaligned __%s_pages start: %#lx end: #%lx\n",
> +				reason, pfn, pfn + nr_pages - 1);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>  /*
>   * Reasonably generic function for adding memory.  It is
>   * expected that archs that support memory hotplug will
>   * call this function after deciding the zone to which to
>   * add the new pages.
>   */
> -int __ref __add_pages(int nid, unsigned long phys_start_pfn,
> -		unsigned long nr_pages, struct mhp_restrictions *restrictions)
> +int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> +		struct mhp_restrictions *restrictions)
>  {
>  	unsigned long i;
> -	int err = 0;
> -	int start_sec, end_sec;
> +	int start_sec, end_sec, err;
>  	struct vmem_altmap *altmap = restrictions->altmap;
>  
> -	/* during initialize mem_map, align hot-added range to section */
> -	start_sec = pfn_to_section_nr(phys_start_pfn);
> -	end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
> -
>  	if (altmap) {
>  		/*
>  		 * Validate altmap is within bounds of the total request
>  		 */
> -		if (altmap->base_pfn != phys_start_pfn
> +		if (altmap->base_pfn != pfn
>  				|| vmem_altmap_offset(altmap) > nr_pages) {
>  			pr_warn_once("memory add fail, invalid altmap\n");
> -			err = -EINVAL;
> -			goto out;
> +			return -EINVAL;
>  		}
>  		altmap->alloc = 0;
>  	}
>  
> +	err = check_pfn_span(pfn, nr_pages, "add");
> +	if (err)
> +		return err;
> +
> +	start_sec = pfn_to_section_nr(pfn);
> +	end_sec = pfn_to_section_nr(pfn + nr_pages - 1);
>  	for (i = start_sec; i <= end_sec; i++) {
> -		err = __add_section(nid, section_nr_to_pfn(i), altmap);
> +		unsigned long pfns;
> +
> +		pfns = min(nr_pages, PAGES_PER_SECTION
> +				- (pfn & ~PAGE_SECTION_MASK));
> +		err = __add_section(nid, pfn, pfns, altmap);
> +		pfn += pfns;
> +		nr_pages -= pfns;
>  
>  		/*
>  		 * EEXIST is finally dealt with by ioresource collision
> @@ -309,7 +342,6 @@ int __ref __add_pages(int nid, unsigned long phys_start_pfn,
>  		cond_resched();
>  	}
>  	vmemmap_populate_print_last();
> -out:
>  	return err;
>  }
>  
> @@ -487,10 +519,10 @@ static void shrink_pgdat_span(struct pglist_data *pgdat,
>  	pgdat->node_spanned_pages = 0;
>  }
>  
> -static void __remove_zone(struct zone *zone, unsigned long start_pfn)
> +static void __remove_zone(struct zone *zone, unsigned long start_pfn,
> +		unsigned long nr_pages)
>  {
>  	struct pglist_data *pgdat = zone->zone_pgdat;
> -	int nr_pages = PAGES_PER_SECTION;
>  	unsigned long flags;
>  
>  	pgdat_resize_lock(zone->zone_pgdat, &flags);
> @@ -499,27 +531,23 @@ static void __remove_zone(struct zone *zone, unsigned long start_pfn)
>  	pgdat_resize_unlock(zone->zone_pgdat, &flags);
>  }
>  
> -static void __remove_section(struct zone *zone, struct mem_section *ms,
> -			     unsigned long map_offset,
> -			     struct vmem_altmap *altmap)
> +static void __remove_section(struct zone *zone, unsigned long pfn,
> +		unsigned long nr_pages, unsigned long map_offset,
> +		struct vmem_altmap *altmap)
>  {
> -	unsigned long start_pfn;
> -	int scn_nr;
> +	struct mem_section *ms = __nr_to_section(pfn_to_section_nr(pfn));
>  
>  	if (WARN_ON_ONCE(!valid_section(ms)))
>  		return;
>  
> -	scn_nr = __section_nr(ms);
> -	start_pfn = section_nr_to_pfn((unsigned long)scn_nr);
> -	__remove_zone(zone, start_pfn);
> -
> -	sparse_remove_one_section(ms, map_offset, altmap);
> +	__remove_zone(zone, pfn, nr_pages);
> +	sparse_remove_one_section(ms, pfn, nr_pages, map_offset, altmap);
>  }
>  
>  /**
>   * __remove_pages() - remove sections of pages from a zone
>   * @zone: zone from which pages need to be removed
> - * @phys_start_pfn: starting pageframe (must be aligned to start of a section)
> + * @pfn: starting pageframe (must be aligned to start of a section)
>   * @nr_pages: number of pages to remove (must be multiple of section size)
>   * @altmap: alternative device page map or %NULL if default memmap is used
>   *
> @@ -528,31 +556,31 @@ static void __remove_section(struct zone *zone, struct mem_section *ms,
>   * sure that pages are marked reserved and zones are adjust properly by
>   * calling offline_pages().
>   */
> -void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> +void __remove_pages(struct zone *zone, unsigned long pfn,
>  		    unsigned long nr_pages, struct vmem_altmap *altmap)
>  {
> -	unsigned long i;
>  	unsigned long map_offset = 0;
> -	int sections_to_remove;
> +	int i, start_sec, end_sec;

As mentioned in v9, use "unsigned long" for start_sec and end_sec please.

-- 

Thanks,

David / dhildenb
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply

* Re: [PATCH] samples: make pidfd-metadata fail gracefully on older kernels
From: Christian Brauner @ 2019-06-20 10:31 UTC (permalink / raw)
  To: Dmitry V. Levin; +Cc: linux-kernel
In-Reply-To: <20190620031144.GA32182@altlinux.org>

On Thu, Jun 20, 2019 at 06:11:44AM +0300, Dmitry V. Levin wrote:
> Initialize pidfd to an invalid descriptor, to fail gracefully on
> those kernels that do not implement CLONE_PIDFD and leave pidfd
> unchanged.
> 
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>  samples/pidfd/pidfd-metadata.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/samples/pidfd/pidfd-metadata.c b/samples/pidfd/pidfd-metadata.c
> index 14b454448429..ff109fdac3a5 100644
> --- a/samples/pidfd/pidfd-metadata.c
> +++ b/samples/pidfd/pidfd-metadata.c
> @@ -83,7 +83,7 @@ static int pidfd_metadata_fd(pid_t pid, int pidfd)
>  
>  int main(int argc, char *argv[])
>  {
> -	int pidfd = 0, ret = EXIT_FAILURE;
> +	int pidfd = -1, ret = EXIT_FAILURE;

Hm, that currently won't work since we added a check in fork.c for
pidfd == 0. It it isn't you'll get EINVAL. This was done to ensure that
we can potentially extend CLONE_PIDFD by passing in flags through the
return argument.
However, I find this increasingly unlikely. Especially since the
interface would be horrendous and an absolute last resort.
If clone3() gets merged for 5.3 (currently in linux-next) we also have
no real need anymore to extend legacy clone() this way. So either wait
until (if) we merge clone3() where the check I mentioned is gone anyway,
or remove the pidfd == 0 check from fork.c in a preliminary patch.
Thoughts?

Thanks!
Christian

^ permalink raw reply

* Re: [PATCH 0/7] clk: at91: sckc: improve error path
From: Claudiu.Beznea @ 2019-06-20 10:30 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: sboyd, mturquette, linux-kernel, claudiu.beznea, linux-clk,
	linux-arm-kernel
In-Reply-To: <20190618095521.GE23549@piout.net>

Hi,

On 18.06.2019 12:55, Alexandre Belloni wrote:
> On 13/06/2019 15:37:06+0000, Claudiu.Beznea@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Hi,
>>
>> This series tries to improve error path for slow clock registrations
>> by adding functions to free resources and using them on failures.
>>
> 
> Does the platform even boot when the slow clock is not available? 
> 
> The TCB clocksource would fail at:
> 
>         tc.slow_clk = of_clk_get_by_name(node->parent, "slow_clk");
>         if (IS_ERR(tc.slow_clk))
>                 return PTR_ERR(tc.slow_clk);
> 

In case of using TC as clocksource, yes, the platform wouldn't boot if slow
clock is not available, because, anyway the TC needs it. PIT may work
without it (if slow clock is not used to drive the PIT).

For sure there are other IPs (which may be or are driven by slow clock)
which may not work if slow clock is driven them.

Anyway, please let me know if you feel this series has no meaning.

Thank you,
Claudiu Beznea

> 
>> It is created on top of patch series at [1].
>>
>> Thank you,
>> Claudiu Beznea
>>
>> [1] https://lore.kernel.org/lkml/1558433454-27971-1-git-send-email-claudiu.beznea@microchip.com/
>>
>> Claudiu Beznea (7):
>>   clk: at91: sckc: add support to free slow oscillator
>>   clk: at91: sckc: add support to free slow rc oscillator
>>   clk: at91: sckc: add support to free slow clock osclillator
>>   clk: at91: sckc: improve error path for sam9x5 sck register
>>   clk: at91: sckc: remove unnecessary line
>>   clk: at91: sckc: improve error path for sama5d4 sck registration
>>   clk: at91: sckc: use dedicated functions to unregister clock
>>
>>  drivers/clk/at91/sckc.c | 122 ++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 86 insertions(+), 36 deletions(-)
>>
>> -- 
>> 2.7.4
>>
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/4] iio: adc: ad7124: Remove input number limitation
From: Caprioru, Mircea @ 2019-06-20 10:30 UTC (permalink / raw)
  To: jic23@kernel.org
  Cc: lars@metafoo.de, Popa, Stefan Serban, linux-iio@vger.kernel.org,
	Hennerich, Michael, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190620091908.12041-1-mircea.caprioru@analog.com>

On Thu, 2019-06-20 at 12:19 +0300, Mircea Caprioru wrote:
> The driver limits the user to use only 4/8 differential inputs, but
> this
> device has the option to use pseudo-differential channels. This will
> increase the number of channels to be equal with the number of inputs
> so 8
> channels for ad7124-4 and 16 for ad7124-8.
> 
> This patch removes the check between channel nodes and num_inputs
> value.
> 
> Signed-off-by: Mircea Caprioru <mircea.caprioru@analog.com>
> ---
>  drivers/iio/adc/ad7124.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index 659ef37d5fe8..810234db9c0d 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -462,13 +462,6 @@ static int ad7124_of_parse_channel_config(struct
> iio_dev *indio_dev,
>  		if (ret)
>  			goto err;
>  
> -		if (ain[0] >= st->chip_info->num_inputs ||
> -		    ain[1] >= st->chip_info->num_inputs) {
> -			dev_err(indio_dev->dev.parent,
> -				"Input pin number out of range.\n");
> -			ret = -EINVAL;
> -			goto err;
> -		}
>  		st->channel_config[channel].ain =
> AD7124_CHANNEL_AINP(ain[0]) |
>  						  AD7124_CHANNEL_AINM(a
> in[1]);
>  		st->channel_config[channel].bipolar =

Please ignore this patch serie, just did a RESEND.

Thanks,
Mircea

^ permalink raw reply

* Re: [PATCH 0/7] clk: at91: sckc: improve error path
From: Claudiu.Beznea @ 2019-06-20 10:30 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: mturquette, sboyd, Nicolas.Ferre, linux-clk, linux-arm-kernel,
	linux-kernel, claudiu.beznea
In-Reply-To: <20190618095521.GE23549@piout.net>

Hi,

On 18.06.2019 12:55, Alexandre Belloni wrote:
> On 13/06/2019 15:37:06+0000, Claudiu.Beznea@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Hi,
>>
>> This series tries to improve error path for slow clock registrations
>> by adding functions to free resources and using them on failures.
>>
> 
> Does the platform even boot when the slow clock is not available? 
> 
> The TCB clocksource would fail at:
> 
>         tc.slow_clk = of_clk_get_by_name(node->parent, "slow_clk");
>         if (IS_ERR(tc.slow_clk))
>                 return PTR_ERR(tc.slow_clk);
> 

In case of using TC as clocksource, yes, the platform wouldn't boot if slow
clock is not available, because, anyway the TC needs it. PIT may work
without it (if slow clock is not used to drive the PIT).

For sure there are other IPs (which may be or are driven by slow clock)
which may not work if slow clock is driven them.

Anyway, please let me know if you feel this series has no meaning.

Thank you,
Claudiu Beznea

> 
>> It is created on top of patch series at [1].
>>
>> Thank you,
>> Claudiu Beznea
>>
>> [1] https://lore.kernel.org/lkml/1558433454-27971-1-git-send-email-claudiu.beznea@microchip.com/
>>
>> Claudiu Beznea (7):
>>   clk: at91: sckc: add support to free slow oscillator
>>   clk: at91: sckc: add support to free slow rc oscillator
>>   clk: at91: sckc: add support to free slow clock osclillator
>>   clk: at91: sckc: improve error path for sam9x5 sck register
>>   clk: at91: sckc: remove unnecessary line
>>   clk: at91: sckc: improve error path for sama5d4 sck registration
>>   clk: at91: sckc: use dedicated functions to unregister clock
>>
>>  drivers/clk/at91/sckc.c | 122 ++++++++++++++++++++++++++++++++++--------------
>>  1 file changed, 86 insertions(+), 36 deletions(-)
>>
>> -- 
>> 2.7.4
>>
> 

^ permalink raw reply

* Re: [PATCH] iwlwifi: add support for hr1 RF ID
From: Luciano Coelho @ 2019-06-20 10:30 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, Oren Givon, stable
In-Reply-To: <20190620084623.12014-1-luca@coelho.fi>

Hi Kalle,

Please take this to 5.1-rc* as well.

--
Cheers,
Luca.


On Thu, 2019-06-20 at 11:46 +0300, Luca Coelho wrote:
> From: Oren Givon <oren.givon@intel.com>
> 
> The 22000 series FW that was meant to be used with hr is
> also the FW that is used for hr1 and has a different RF ID.
> Add support to load the hr FW when hr1 RF ID is detected.
> 
> Cc: stable@vger.kernel.org # 5.1+
> Signed-off-by: Oren Givon <oren.givon@intel.com>
> Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
> ---
>  drivers/net/wireless/intel/iwlwifi/iwl-csr.h    | 1 +
>  drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 8 +++++---
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-csr.h b/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
> index 553554846009..93da96a7247c 100644
> --- a/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
> +++ b/drivers/net/wireless/intel/iwlwifi/iwl-csr.h
> @@ -336,6 +336,7 @@ enum {
>  /* RF_ID value */
>  #define CSR_HW_RF_ID_TYPE_JF		(0x00105100)
>  #define CSR_HW_RF_ID_TYPE_HR		(0x0010A000)
> +#define CSR_HW_RF_ID_TYPE_HR1		(0x0010c100)
>  #define CSR_HW_RF_ID_TYPE_HRCDB		(0x00109F00)
>  #define CSR_HW_RF_ID_TYPE_GF		(0x0010D000)
>  #define CSR_HW_RF_ID_TYPE_GF4		(0x0010E000)
> diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> index b93753233223..38ab24d96244 100644
> --- a/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> +++ b/drivers/net/wireless/intel/iwlwifi/pcie/trans.c
> @@ -3575,9 +3575,11 @@ struct iwl_trans *iwl_trans_pcie_alloc(struct pci_dev *pdev,
>  			trans->cfg = &iwlax411_2ax_cfg_so_gf4_a0;
>  		}
>  	} else if (cfg == &iwl_ax101_cfg_qu_hr) {
> -		if (CSR_HW_RF_ID_TYPE_CHIP_ID(trans->hw_rf_id) ==
> -		    CSR_HW_RF_ID_TYPE_CHIP_ID(CSR_HW_RF_ID_TYPE_HR) &&
> -		    trans->hw_rev == CSR_HW_REV_TYPE_QNJ_B0) {
> +		if ((CSR_HW_RF_ID_TYPE_CHIP_ID(trans->hw_rf_id) ==
> +		     CSR_HW_RF_ID_TYPE_CHIP_ID(CSR_HW_RF_ID_TYPE_HR) &&
> +		     trans->hw_rev == CSR_HW_REV_TYPE_QNJ_B0) ||
> +		    (CSR_HW_RF_ID_TYPE_CHIP_ID(trans->hw_rf_id) ==
> +		     CSR_HW_RF_ID_TYPE_CHIP_ID(CSR_HW_RF_ID_TYPE_HR1))) {
>  			trans->cfg = &iwl22000_2ax_cfg_qnj_hr_b0;
>  		} else if (CSR_HW_RF_ID_TYPE_CHIP_ID(trans->hw_rf_id) ==
>  		    CSR_HW_RF_ID_TYPE_CHIP_ID(CSR_HW_RF_ID_TYPE_HR)) {


^ permalink raw reply

* Re: linux-next: build failure after merge of the nvdimm tree
From: Pankaj Gupta @ 2019-06-20 10:30 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Dan Williams, Linux Next Mailing List, Linux Kernel Mailing List,
	Mike Snitzer
In-Reply-To: <20190620192939.139bf5c0@canb.auug.org.au>


> Hi Dan,
> 
> After merging the nvdimm tree, today's linux-next build (i386 defconfig)
> failed like this:
> 
> drivers/md/dm-table.c: In function 'device_synchronous':
> drivers/md/dm-table.c:897:9: error: implicit declaration of function
> 'dax_synchronous'; did you mean 'device_synchronous'?
> [-Werror=implicit-function-declaration]
>   return dax_synchronous(dev->dax_dev);
>          ^~~~~~~~~~~~~~~
>          device_synchronous
> drivers/md/dm-table.c: In function 'dm_table_set_restrictions':
> drivers/md/dm-table.c:1925:4: error: implicit declaration of function
> 'set_dax_synchronous'; did you mean 'device_synchronous'?
> [-Werror=implicit-function-declaration]
>     set_dax_synchronous(t->md->dax_dev);
>     ^~~~~~~~~~~~~~~~~~~
>     device_synchronous
> cc1: some warnings being treated as errors
> 
> Caused by commit
> 
>   38887edec247 ("dm: enable synchronous dax")
> 
> CONFIG_DAX is not set for this build.

Moving 'dax_synchronous' & 'set_dax_synchronous' function declaration
out of CONFIG_DAX should fix this. This will make compiler aware about 
'device_synchronous' function for non DAX archs as well.

Thanks,
Pankaj
 
> 
> I have reverted that commit for today.
> --
> Cheers,
> Stephen Rothwell
> 

^ permalink raw reply

* Re: [PATCH V9] i2c: tegra: remove BUG() macro
From: Thierry Reding @ 2019-06-20 10:30 UTC (permalink / raw)
  To: Bitan Biswas
  Cc: Laxman Dewangan, Thierry Reding, Jonathan Hunter, linux-i2c,
	linux-tegra, linux-kernel, Peter Rosin, Wolfram Sang,
	Dmitry Osipenko, Shardar Mohammed, Sowjanya Komatineni,
	Mantravadi Karthik
In-Reply-To: <1560856182-26072-1-git-send-email-bbiswas@nvidia.com>

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

On Tue, Jun 18, 2019 at 04:09:42AM -0700, Bitan Biswas wrote:
> The usage of BUG() macro is generally discouraged in kernel, unless
> it's a problem that results in a physical damage or loss of data.
> This patch removes unnecessary BUG() macros and replaces the rest
> with warning.
> 
> Signed-off-by: Bitan Biswas <bbiswas@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c | 47 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 8 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply

* Re: [dpdk-dev] [PATCH 01/28] vhost: introduce vhost transport operations structure
From: Bruce Richardson @ 2019-06-20 10:30 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Nikos Dragazis, dev, Maxime Coquelin, Tiwei Bie, Zhihong Wang,
	Stefan Hajnoczi, Wei Wang, Stojaczyk Dariusz, Vangelis Koukis
In-Reply-To: <f7th88lnxim.fsf@dhcp-25.97.bos.redhat.com>

On Wed, Jun 19, 2019 at 04:14:09PM -0400, Aaron Conole wrote:
> Nikos Dragazis <ndragazis@arrikto.com> writes:
> 
> > This is the first of a series of patches, whose purpose is to add
> > support for the virtio-vhost-user transport. This is a vhost-user
> > transport implementation that is different from the default AF_UNIX
> > transport. It uses the virtio-vhost-user PCI device in order to tunnel
> > vhost-user protocol messages over virtio. This lets guests act as vhost
> > device backends for other guests.
> >
> > File descriptor passing is specific to the AF_UNIX vhost-user protocol
> > transport.  In order to add support for additional transports, it is
> > necessary to extract transport-specific code from the main vhost-user
> > code.
> >
> > This patch introduces struct vhost_transport_ops and associates each
> > device with a transport.  Core vhost-user code calls into
> > vhost_transport_ops to perform transport-specific operations.
> >
> > Notifying callfd is a transport-specific operation, so it belongs to
> > trans_af_unix.c.
> >
> > Several more patches follow this one to complete the task of moving
> > AF_UNIX transport code out of core vhost-user code.
> >
> > Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> 
> You'll need to also accommodate the meson build - probably with
> something like:
> 
> diff --git a/lib/librte_vhost/meson.build b/lib/librte_vhost/meson.build
> index 3090bbe08..81b70683b 100644
> --- a/lib/librte_vhost/meson.build
> +++ b/lib/librte_vhost/meson.build
> @@ -14,6 +14,6 @@ allow_experimental_apis = true
>  cflags += '-fno-strict-aliasing'
>  sources = files('fd_man.c', 'iotlb.c', 'socket.c', 'vdpa.c',
>                 'vhost.c', 'vhost_user.c',
> -               'virtio_net.c', 'vhost_crypto.c')
> +               'virtio_net.c', 'vhost_crypto.c', 'trans_af_unix.c')
>  headers = files('rte_vhost.h', 'rte_vdpa.h', 'rte_vhost_crypto.h')
>  deps += ['ethdev', 'cryptodev', 'hash', 'pci']
> 
> 

Yep, except I think we should try and keep the files in alphabetical order,
with only a couple of entries per line [so place trans_af_unix.c on a new
line with vdpa.c].

^ permalink raw reply

* Re: [PATCH] [RFC] dmaengine: add fifo_size member
From: Sameer Pujar @ 2019-06-20 10:29 UTC (permalink / raw)
  To: Vinod Koul
  Cc: dan.j.williams, tiwai, jonathanh, dmaengine, linux-kernel,
	sharadg, rlokhande, dramesh, mkumard
In-Reply-To: <20190618043308.GJ2962@vkoul-mobl>


On 6/18/2019 10:03 AM, Vinod Koul wrote:
> On 17-06-19, 12:37, Sameer Pujar wrote:
>> On 6/13/2019 10:13 AM, Vinod Koul wrote:
>>> On 06-06-19, 09:19, Sameer Pujar wrote:
>>>
>>>>> you are really going other way around about the whole picture. FWIW that
>>>>> is how *other* folks do audio with dmaengine!
>>>> I discussed this internally with HW folks and below is the reason why DMA
>>>> needs
>>>> to know FIFO size.
>>>>
>>>> - FIFOs reside in peripheral device(ADMAIF), which is the ADMA interface to
>>>> the audio sub-system.
>>>> - ADMAIF has multiple channels and share FIFO buffer for individual
>>>> operations. There is a provision
>>>>     to allocate specific fifo size for each individual ADMAIF channel from the
>>>> shared buffer.
>>>> - Tegra Audio DMA(ADMA) architecture is different from the usual DMA
>>>> engines, which you described earlier.
>>>> - The flow control logic is placed inside ADMA. Slave peripheral
>>>> device(ADMAIF) signals ADMA whenever a
>>>>     read or write happens on the FIFO(per WORD basis). Please note that the
>>>> signaling is per channel. There is
>>>>     no other signaling present from ADMAIF to ADMA.
>>>> - ADMA keeps a counter related to above signaling. Whenever a sufficient
>>> when is signal triggered? When there is space available or some
>>> threshold of space is reached?
>> Signal is triggered when FIFO read/write happens on the peripheral side. In
>> other words
>> this happens when data is pushed/popped out of ADMAIF from/to one of the
>> AHUB modules (I2S
>> for example) This is on peripheral side and ADMAIF signals ADMA per WORD
>> basis.
>> ADMA <---(1. DMA transfers)---> ADMAIF <------ (2. FIFO read/write) ------>
>> I2S
>> To be more clear ADMAIF signals ADMA when [2] happens.
> That is on every word read/write?
yes this per WORD read/write
>
>> FIFO_THRESHOLD field in ADMAIF is just to indicate when can ADMAIF do
>> operation [2].
>> Also please note FIFO_THRESHOLD field is present only for memory---->AHUB
>> path (playback path)
>> and there is no such threshold concept for AHUB----> memory path (capture
>> path)
> That is sane and common. For memory you dont have a constraint so you
> transfer at full throttle.
>
>>>> space is available, it initiates a transfer.
>>>>     But the question is, how does it know when to transfer. This is the
>>>> reason, why ADMA has to be aware of FIFO
>>>>     depth of ADMAIF channel. Depending on the counters and FIFO depth, it
>>>> knows exactly when a free space is available
>>>>     in the context of a specific channel. On ADMA, FIFO_SIZE is just a value
>>>> which should match to actual FIFO_DEPTH/SIZE
>>>>     of ADMAIF channel.
>>> That doesn't sound too different from typical dmaengine. To give an
>>> example of a platform (and general DMAengine principles as well) I worked
>>> on the FIFO was 16 word deep. DMA didn't knew!
>>>
>>> Peripheral driver would signal to DMA when a threshold is reached and
>> No, In our case ADMAIF does not do any threshold based signalling to ADMA.
>>> DMA would send a burst controlled by src/dst_burst_size. For example if
>>> you have a FIFO with 16 words depth, typical burst_size would be 8 words
>>> and peripheral will configure signalling for FIFO having 8 words, so
>>> signal from peripheral will make dma transfer 8 words.
>> The scenario is different in ADMA case, as ADMAIF cannot configure the
>> signalling based on FIFO_THRESHOLD settings.
>>> Here the peripheral driver FIFO is important, but the driver
>>> configures it and sets burst_size accordingly.
>>>
>>> So can you explain me what is the difference here that the peripheral
>>> cannot configure and use burst size with passing fifo depth?
>> Say for example FIFO_THRESHOLD is programmed as 16 WORDS, BURST_SIZE as 8
>> WORDS.
>> ADMAIF does not push data to AHUB(operation [2]) till threshold of 16 WORDS
>> is
>> reached in ADMAIF FIFO. Hence 2 burst transfers are needed to reach the
>> threshold.
>> As mentioned earlier, threshold here is to just indicate when data transfer
>> can happen
>> to AHUB modules.
> So we have ADMA and AHUB and peripheral. You are talking to AHUB and that
> is _not_ peripheral and if I have guess right the fifo depth is for AHUB
> right?
Yes the communication is between ADMA and AHUB. ADMAIF is the interface 
between
ADMA and AHUB. ADMA channel sending data to AHUB pairs with ADMAIF TX 
channel.
Similary ADMA channel receiving data from AHUB pairs with ADMAIF RX channel.
FIFO DEPTH we are talking is about each ADMAIF TX/RX channel and it is 
configurable.
DMA transfers happen to/from ADMAIF FIFOs and whenever data(per WORD) is 
popped/pushed
out of ADMAIF to/from AHUB, asseration is made to ADMA. ADMA keeps 
counters based on
these assertions. By knowing FIFO DEPTH and these counters, ADMA knows 
when to wait or
when to transfer data.
>
>> Once the data is popped from ADMAIF FIFO, ADMAIF signals ADMA. ADMA is the
>> master
>> and it keeps track of the buffer occupancy by knowing the FIFO_DEPTH and the
>> signalling.
>> Then finally it decides when to do next burst transfer depending on the free
>> space
>> available in ADMAIF.
>>>> - Now consider two cases based on above logic,
>>>>     * Case 1: when DMA_FIFO_SIZE > SLAVE_FIFO_SIZE
>>>>       In this case, ADMA thinks that there is enough space available for
>>>> transfer, when actually the FIFO data
>>>>       on slave is not consumed yet. It would result in OVERRUN.
>>>>     * Case 2: when DMA_FIFO_SIZE < SLAVE_FIFO_SIZE
>>>>       This is case where ADMA won’t transfer, even though sufficient space is
>>>> available, resulting in UNDERRUN.
>>>> - The guideline is to program, DMA_FIFO_SIZE(on ADMA side) =
>>>> SLAVE_FIFO_SIZE(on ADMAIF side) and hence we need a
>>>>     way to communicate fifo size info to ADMA.

^ permalink raw reply

* Re: [PATCH v1 4/5] coresight: etm4x: improve clarity of etm4_os_unlock comment
From: Andrew Murray @ 2019-06-20 10:29 UTC (permalink / raw)
  To: Suzuki K Poulose; +Cc: alexander.shishkin, linux-arm-kernel, mathieu.poirier
In-Reply-To: <c4064f38-9699-219e-706e-2fdfa8277a62@arm.com>

On Wed, Jun 19, 2019 at 11:46:07AM +0100, Suzuki K Poulose wrote:
> 
> 
> On 18/06/2019 13:54, Andrew Murray wrote:
> > To improve clarity, let's update the comment for etm4_os_unlock
> > to use the name of the register as per the ETM architecture
> > specification.
> 
> nit: It also "fixes" the comment. Writing any value doesn't unlock
> the trace unit. It must be "0".
> 
> > 
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> >   drivers/hwtracing/coresight/coresight-etm4x.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> > index 885c4f2c691c..bda90d4cd62b 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> > @@ -47,7 +47,7 @@ static enum cpuhp_state hp_online;
> >   static void etm4_os_unlock(struct etmv4_drvdata *drvdata)
> >   {
> > -	/* Writing any value to ETMOSLAR unlocks the trace registers */
> > +	/* Writing 0 to TRCOSLAR unlocks the trace registers */
> >   	writel_relaxed(0x0, drvdata->base + TRCOSLAR);
> >   	drvdata->os_unlock = true;
> >   	isb();
> 
> With the above :
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Thanks, I'll update the message.

Andrew Murray

_______________________________________________
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 06/10] mfd / platform: cros_ec: Reorganize platform and mfd includes
From: Chanwoo Choi @ 2019-06-20 10:28 UTC (permalink / raw)
  To: Enric Balletbo i Serra, linux-kernel
  Cc: gwendal, Guenter Roeck, Benson Leung, Lee Jones, kernel, dtor,
	Andy Shevchenko, Mark Brown, Wolfram Sang, Neil Armstrong,
	Alexandre Belloni, Benjamin Tissoires, Dmitry Torokhov,
	Sebastian Reichel, Mauro Carvalho Chehab, alsa-devel,
	Alessandro Zummo, linux-iio, Fabien Lahoudere, linux-i2c,
	linux-rt
In-Reply-To: <20190614163635.22413-7-enric.balletbo@collabora.com>

Hi Enric,

For extcon part,
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Best Regards,
Chanwoo choi

On 19. 6. 15. 오전 1:36, Enric Balletbo i Serra wrote:
> There is a bit of mess between cros-ec mfd includes and platform
> includes. For example, we have a linux/mfd/cros_ec.h include that
> exports the interface implemented in platform/chrome/cros_ec_proto.c. Or
> we have a linux/mfd/cros_ec_commands.h file that is non related to the
> multifunction device (in the sense that is not exporting any function of
> the mfd device). This causes crossed includes between mfd and
> platform/chrome subsystems and makes the code difficult to read, apart
> from creating 'curious' situations where a platform/chrome driver includes
> a linux/mfd/cros_ec.h file just to get the exported functions that are
> implemented in another platform/chrome driver.
> 
> In order to have a better separation on what the cros-ec multifunction
> driver does and what the cros-ec core provides move and rework the
> affected includes doing:
> 
>  - Move cros_ec_commands.h to include/linux/platform_data/cros_ec_commands.h
>  - Get rid of the parts that are implemented in the platform/chrome/cros_ec_proto.c
>    driver from include/linux/mfd/cros_ec.h to a new file
>    include/linux/platform_data/cros_ec_proto.h
>  - Update all the drivers with the new includes, so
>    - Drivers that only need to know about the protocol include
>      - linux/platform_data/cros_ec_proto.h
>      - linux/platform_data/cros_ec_commands.h
>    - Drivers that need to know about the cros-ec mfd device also include
>      - linux/mfd/cros_ec.h
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Mark Brown <broonie@kernel.org>
> Acked-by: Wolfram Sang <wsa@the-dreams.de>
> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> 
> Changes in v2: None
> 
>  drivers/extcon/extcon-usbc-cros-ec.c          |   3 +-
>  drivers/hid/hid-google-hammer.c               |   4 +-
>  drivers/i2c/busses/i2c-cros-ec-tunnel.c       |   4 +-
>  drivers/iio/accel/cros_ec_accel_legacy.c      |   3 +-
>  .../common/cros_ec_sensors/cros_ec_sensors.c  |   3 +-
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |   3 +-
>  drivers/iio/light/cros_ec_light_prox.c        |   3 +-
>  drivers/iio/pressure/cros_ec_baro.c           |   3 +-
>  drivers/input/keyboard/cros_ec_keyb.c         |   4 +-
>  .../media/platform/cros-ec-cec/cros-ec-cec.c  |   4 +-
>  drivers/mfd/cros_ec_dev.c                     |   3 +-
>  drivers/platform/chrome/cros_ec.c             |   3 +-
>  drivers/platform/chrome/cros_ec_debugfs.c     |   3 +-
>  drivers/platform/chrome/cros_ec_i2c.c         |   4 +-
>  drivers/platform/chrome/cros_ec_lightbar.c    |   3 +-
>  drivers/platform/chrome/cros_ec_lpc.c         |   4 +-
>  drivers/platform/chrome/cros_ec_lpc_reg.c     |   4 +-
>  drivers/platform/chrome/cros_ec_proto.c       |   3 +-
>  drivers/platform/chrome/cros_ec_rpmsg.c       |   4 +-
>  drivers/platform/chrome/cros_ec_spi.c         |   4 +-
>  drivers/platform/chrome/cros_ec_sysfs.c       |   3 +-
>  drivers/platform/chrome/cros_ec_trace.c       |   2 +-
>  drivers/platform/chrome/cros_ec_trace.h       |   4 +-
>  drivers/platform/chrome/cros_ec_vbc.c         |   3 +-
>  drivers/platform/chrome/cros_usbpd_logger.c   |   5 +-
>  drivers/power/supply/cros_usbpd-charger.c     |   5 +-
>  drivers/pwm/pwm-cros-ec.c                     |   4 +-
>  drivers/rtc/rtc-cros-ec.c                     |   3 +-
>  .../linux/iio/common/cros_ec_sensors_core.h   |   3 +-
>  include/linux/mfd/cros_ec.h                   | 306 -----------------
>  .../{mfd => platform_data}/cros_ec_commands.h |   0
>  include/linux/platform_data/cros_ec_proto.h   | 315 ++++++++++++++++++
>  sound/soc/codecs/cros_ec_codec.c              |   4 +-
>  33 files changed, 377 insertions(+), 349 deletions(-)
>  rename include/linux/{mfd => platform_data}/cros_ec_commands.h (100%)
>  create mode 100644 include/linux/platform_data/cros_ec_proto.h
> 
> diff --git a/drivers/extcon/extcon-usbc-cros-ec.c b/drivers/extcon/extcon-usbc-cros-ec.c
> index 43c0a936ab82..5290cc2d19d9 100644
> --- a/drivers/extcon/extcon-usbc-cros-ec.c
> +++ b/drivers/extcon/extcon-usbc-cros-ec.c
> @@ -6,10 +6,11 @@
>  
>  #include <linux/extcon-provider.h>
>  #include <linux/kernel.h>
> -#include <linux/mfd/cros_ec.h>
>  #include <linux/module.h>
>  #include <linux/notifier.h>
>  #include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/sched.h>
> diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
> index ee5e0bdcf078..84f8c127ebdc 100644
> --- a/drivers/hid/hid-google-hammer.c
> +++ b/drivers/hid/hid-google-hammer.c
> @@ -16,9 +16,9 @@
>  #include <linux/acpi.h>
>  #include <linux/hid.h>
>  #include <linux/leds.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_wakeup.h>
>  #include <asm/unaligned.h>
> diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> index 82bcd9a78759..c551aa96a2e3 100644
> --- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> +++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
> @@ -5,8 +5,8 @@
>  
>  #include <linux/module.h>
>  #include <linux/i2c.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c b/drivers/iio/accel/cros_ec_accel_legacy.c
> index 46bb2e421bb9..fd9a634f741e 100644
> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> @@ -18,9 +18,10 @@
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  
>  #define DRV_NAME	"cros-ec-accel-legacy"
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> index 17af4e0fd5f8..40dc24ff0ee5 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -17,8 +17,9 @@
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 719a0df5aeeb..fd63315399ac 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -14,9 +14,10 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  
>  static char *cros_ec_loc[] = {
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index 308ee6ff2e22..437e0eae9178 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -15,8 +15,9 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index 034ce98d6e97..956dc01f1295 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -15,9 +15,10 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  
>  /*
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index d56001181598..2b71c5a51f90 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -22,8 +22,8 @@
>  #include <linux/slab.h>
>  #include <linux/sysrq.h>
>  #include <linux/input/matrix_keypad.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #include <asm/unaligned.h>
>  
> diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> index 068df9888dbf..2e4e263a4a94 100644
> --- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> +++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> @@ -16,8 +16,8 @@
>  #include <linux/interrupt.h>
>  #include <media/cec.h>
>  #include <media/cec-notifier.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #define DRV_NAME	"cros-ec-cec"
>  
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index d465bcde9fc4..7572fe096c72 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -19,11 +19,12 @@
>  
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/slab.h>
>  
>  #define DRV_NAME "cros-ec-dev"
> diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
> index 11fced7917fc..9800597ccd96 100644
> --- a/drivers/platform/chrome/cros_ec.c
> +++ b/drivers/platform/chrome/cros_ec.c
> @@ -21,7 +21,8 @@
>  #include <linux/interrupt.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/suspend.h>
>  #include <asm/unaligned.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
> index 4c2a27f6a6d0..b088d91be9c9 100644
> --- a/drivers/platform/chrome/cros_ec_debugfs.c
> +++ b/drivers/platform/chrome/cros_ec_debugfs.c
> @@ -8,9 +8,10 @@
>  #include <linux/delay.h>
>  #include <linux/fs.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/poll.h>
>  #include <linux/sched.h>
> diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
> index 6bb82dfa7dae..9bd97bc8454b 100644
> --- a/drivers/platform/chrome/cros_ec_i2c.c
> +++ b/drivers/platform/chrome/cros_ec_i2c.c
> @@ -9,8 +9,8 @@
>  #include <linux/module.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index d30a6650b0b5..caa26da2c788 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -9,8 +9,9 @@
>  #include <linux/fs.h>
>  #include <linux/kobject.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/sched.h>
>  #include <linux/types.h>
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 2c7e654cf89c..0c976e95998a 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -16,9 +16,9 @@
>  #include <linux/delay.h>
>  #include <linux/io.h>
>  #include <linux/interrupt.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>  #include <linux/suspend.h>
> diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
> index 0f5cd0ac8b49..dec9a779e209 100644
> --- a/drivers/platform/chrome/cros_ec_lpc_reg.c
> +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
> @@ -4,8 +4,8 @@
>  // Copyright (C) 2016 Google, Inc
>  
>  #include <linux/io.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #include "cros_ec_lpc_mec.h"
>  
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index 3d2325197a68..f659f96bda12 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -3,10 +3,11 @@
>  //
>  // Copyright (C) 2015 Google, Inc
>  
> -#include <linux/mfd/cros_ec.h>
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/slab.h>
>  #include <asm/unaligned.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
> index 520e507bfa54..9633e5417686 100644
> --- a/drivers/platform/chrome/cros_ec_rpmsg.c
> +++ b/drivers/platform/chrome/cros_ec_rpmsg.c
> @@ -6,9 +6,9 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/rpmsg.h>
>  #include <linux/slab.h>
> diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
> index 02f9e8257581..a4167dfd85bf 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -6,9 +6,9 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/of.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
> diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
> index fe0b7614ae1b..0caeb8d0989d 100644
> --- a/drivers/platform/chrome/cros_ec_sysfs.c
> +++ b/drivers/platform/chrome/cros_ec_sysfs.c
> @@ -9,8 +9,9 @@
>  #include <linux/fs.h>
>  #include <linux/kobject.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/printk.h>
>  #include <linux/slab.h>
> diff --git a/drivers/platform/chrome/cros_ec_trace.c b/drivers/platform/chrome/cros_ec_trace.c
> index 0a76412095a9..6f80ff4532ae 100644
> --- a/drivers/platform/chrome/cros_ec_trace.c
> +++ b/drivers/platform/chrome/cros_ec_trace.c
> @@ -6,7 +6,7 @@
>  #define TRACE_SYMBOL(a) {a, #a}
>  
>  // Generate the list using the following script:
> -// sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1), \\/p' include/linux/mfd/cros_ec_commands.h
> +// sed -n 's/^#define \(EC_CMD_[[:alnum:]_]*\)\s.*/\tTRACE_SYMBOL(\1), \\/p' include/linux/platform_data/cros_ec_commands.h
>  #define EC_CMDS \
>  	TRACE_SYMBOL(EC_CMD_PROTO_VERSION), \
>  	TRACE_SYMBOL(EC_CMD_HELLO), \
> diff --git a/drivers/platform/chrome/cros_ec_trace.h b/drivers/platform/chrome/cros_ec_trace.h
> index 7ae3b89c78b9..0dd4df30fa89 100644
> --- a/drivers/platform/chrome/cros_ec_trace.h
> +++ b/drivers/platform/chrome/cros_ec_trace.h
> @@ -11,8 +11,10 @@
>  #if !defined(_CROS_EC_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
>  #define _CROS_EC_TRACE_H_
>  
> +#include <linux/bits.h>
>  #include <linux/types.h>
> -#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  #include <linux/tracepoint.h>
>  
> diff --git a/drivers/platform/chrome/cros_ec_vbc.c b/drivers/platform/chrome/cros_ec_vbc.c
> index 8392a1ec33a7..cffe119e7a7a 100644
> --- a/drivers/platform/chrome/cros_ec_vbc.c
> +++ b/drivers/platform/chrome/cros_ec_vbc.c
> @@ -7,8 +7,9 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/slab.h>
>  
>  #define DRV_NAME "cros-ec-vbc"
> diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c
> index 7c7b267626a0..c549a9b49b56 100644
> --- a/drivers/platform/chrome/cros_usbpd_logger.c
> +++ b/drivers/platform/chrome/cros_usbpd_logger.c
> @@ -6,10 +6,11 @@
>   */
>  
>  #include <linux/ktime.h>
> -#include <linux/math64.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtc.h>
>  
> diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
> index 7e9c3984ef6a..ed8eca28c195 100644
> --- a/drivers/power/supply/cros_usbpd-charger.c
> +++ b/drivers/power/supply/cros_usbpd-charger.c
> @@ -5,9 +5,10 @@
>   * Copyright (c) 2014 - 2018 Google, Inc
>   */
>  
> -#include <linux/module.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
>  #include <linux/slab.h>
> diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
> index 98f6ac6cf6ab..85bea2d40b7d 100644
> --- a/drivers/pwm/pwm-cros-ec.c
> +++ b/drivers/pwm/pwm-cros-ec.c
> @@ -6,8 +6,8 @@
>   */
>  
>  #include <linux/module.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/pwm.h>
>  #include <linux/slab.h>
> diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
> index 4d6bf9304ceb..6909e01936d9 100644
> --- a/drivers/rtc/rtc-cros-ec.c
> +++ b/drivers/rtc/rtc-cros-ec.c
> @@ -6,8 +6,9 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtc.h>
>  #include <linux/slab.h>
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index ce16445411ac..8a91669f5bed 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -18,7 +18,8 @@
>  
>  #include <linux/iio/iio.h>
>  #include <linux/irqreturn.h>
> -#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  
>  enum {
>  	CROS_EC_SENSOR_X,
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 2a1372d167b9..e0bae49535e1 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -16,184 +16,7 @@
>  #ifndef __LINUX_MFD_CROS_EC_H
>  #define __LINUX_MFD_CROS_EC_H
>  
> -#include <linux/cdev.h>
>  #include <linux/device.h>
> -#include <linux/notifier.h>
> -#include <linux/mfd/cros_ec_commands.h>
> -#include <linux/mutex.h>
> -
> -#define CROS_EC_DEV_NAME "cros_ec"
> -#define CROS_EC_DEV_FP_NAME "cros_fp"
> -#define CROS_EC_DEV_PD_NAME "cros_pd"
> -#define CROS_EC_DEV_TP_NAME "cros_tp"
> -#define CROS_EC_DEV_ISH_NAME "cros_ish"
> -
> -/*
> - * The EC is unresponsive for a time after a reboot command.  Add a
> - * simple delay to make sure that the bus stays locked.
> - */
> -#define EC_REBOOT_DELAY_MS             50
> -
> -/*
> - * Max bus-specific overhead incurred by request/responses.
> - * I2C requires 1 additional byte for requests.
> - * I2C requires 2 additional bytes for responses.
> - * SPI requires up to 32 additional bytes for responses.
> - */
> -#define EC_PROTO_VERSION_UNKNOWN	0
> -#define EC_MAX_REQUEST_OVERHEAD		1
> -#define EC_MAX_RESPONSE_OVERHEAD	32
> -
> -/*
> - * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
> - */
> -enum {
> -	EC_MSG_TX_HEADER_BYTES	= 3,
> -	EC_MSG_TX_TRAILER_BYTES	= 1,
> -	EC_MSG_TX_PROTO_BYTES	= EC_MSG_TX_HEADER_BYTES +
> -					EC_MSG_TX_TRAILER_BYTES,
> -	EC_MSG_RX_PROTO_BYTES	= 3,
> -
> -	/* Max length of messages for proto 2*/
> -	EC_PROTO2_MSG_BYTES		= EC_PROTO2_MAX_PARAM_SIZE +
> -					EC_MSG_TX_PROTO_BYTES,
> -
> -	EC_MAX_MSG_BYTES		= 64 * 1024,
> -};
> -
> -/**
> - * struct cros_ec_command - Information about a ChromeOS EC command.
> - * @version: Command version number (often 0).
> - * @command: Command to send (EC_CMD_...).
> - * @outsize: Outgoing length in bytes.
> - * @insize: Max number of bytes to accept from the EC.
> - * @result: EC's response to the command (separate from communication failure).
> - * @data: Where to put the incoming data from EC and outgoing data to EC.
> - */
> -struct cros_ec_command {
> -	uint32_t version;
> -	uint32_t command;
> -	uint32_t outsize;
> -	uint32_t insize;
> -	uint32_t result;
> -	uint8_t data[0];
> -};
> -
> -/**
> - * struct cros_ec_device - Information about a ChromeOS EC device.
> - * @phys_name: Name of physical comms layer (e.g. 'i2c-4').
> - * @dev: Device pointer for physical comms device
> - * @was_wake_device: True if this device was set to wake the system from
> - *                   sleep at the last suspend.
> - * @cros_class: The class structure for this device.
> - * @cmd_readmem: Direct read of the EC memory-mapped region, if supported.
> - *     @offset: Is within EC_LPC_ADDR_MEMMAP region.
> - *     @bytes: Number of bytes to read. zero means "read a string" (including
> - *             the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be
> - *             read. Caller must ensure that the buffer is large enough for the
> - *             result when reading a string.
> - * @max_request: Max size of message requested.
> - * @max_response: Max size of message response.
> - * @max_passthru: Max sice of passthru message.
> - * @proto_version: The protocol version used for this device.
> - * @priv: Private data.
> - * @irq: Interrupt to use.
> - * @id: Device id.
> - * @din: Input buffer (for data from EC). This buffer will always be
> - *       dword-aligned and include enough space for up to 7 word-alignment
> - *       bytes also, so we can ensure that the body of the message is always
> - *       dword-aligned (64-bit). We use this alignment to keep ARM and x86
> - *       happy. Probably word alignment would be OK, there might be a small
> - *       performance advantage to using dword.
> - * @dout: Output buffer (for data to EC). This buffer will always be
> - *        dword-aligned and include enough space for up to 7 word-alignment
> - *        bytes also, so we can ensure that the body of the message is always
> - *        dword-aligned (64-bit). We use this alignment to keep ARM and x86
> - *        happy. Probably word alignment would be OK, there might be a small
> - *        performance advantage to using dword.
> - * @din_size: Size of din buffer to allocate (zero to use static din).
> - * @dout_size: Size of dout buffer to allocate (zero to use static dout).
> - * @wake_enabled: True if this device can wake the system from sleep.
> - * @suspended: True if this device had been suspended.
> - * @cmd_xfer: Send command to EC and get response.
> - *            Returns the number of bytes received if the communication
> - *            succeeded, but that doesn't mean the EC was happy with the
> - *            command. The caller should check msg.result for the EC's result
> - *            code.
> - * @pkt_xfer: Send packet to EC and get response.
> - * @lock: One transaction at a time.
> - * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
> - * @host_sleep_v1: True if this EC supports the sleep v1 command.
> - * @event_notifier: Interrupt event notifier for transport devices.
> - * @event_data: Raw payload transferred with the MKBP event.
> - * @event_size: Size in bytes of the event data.
> - * @host_event_wake_mask: Mask of host events that cause wake from suspend.
> - * @ec: The platform_device used by the mfd driver to interface with the
> - *      main EC.
> - * @pd: The platform_device used by the mfd driver to interface with the
> - *      PD behind an EC.
> - */
> -struct cros_ec_device {
> -	/* These are used by other drivers that want to talk to the EC */
> -	const char *phys_name;
> -	struct device *dev;
> -	bool was_wake_device;
> -	struct class *cros_class;
> -	int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
> -			   unsigned int bytes, void *dest);
> -
> -	/* These are used to implement the platform-specific interface */
> -	u16 max_request;
> -	u16 max_response;
> -	u16 max_passthru;
> -	u16 proto_version;
> -	void *priv;
> -	int irq;
> -	u8 *din;
> -	u8 *dout;
> -	int din_size;
> -	int dout_size;
> -	bool wake_enabled;
> -	bool suspended;
> -	int (*cmd_xfer)(struct cros_ec_device *ec,
> -			struct cros_ec_command *msg);
> -	int (*pkt_xfer)(struct cros_ec_device *ec,
> -			struct cros_ec_command *msg);
> -	struct mutex lock;
> -	bool mkbp_event_supported;
> -	bool host_sleep_v1;
> -	struct blocking_notifier_head event_notifier;
> -
> -	struct ec_response_get_next_event_v1 event_data;
> -	int event_size;
> -	u32 host_event_wake_mask;
> -
> -	/* The platform devices used by the mfd driver */
> -	struct platform_device *ec;
> -	struct platform_device *pd;
> -};
> -
> -/**
> - * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information.
> - * @sensor_num: Id of the sensor, as reported by the EC.
> - */
> -struct cros_ec_sensor_platform {
> -	u8 sensor_num;
> -};
> -
> -/**
> - * struct cros_ec_platform - ChromeOS EC platform information.
> - * @ec_name: Name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
> - *           used in /dev/ and sysfs.
> - * @cmd_offset: Offset to apply for each command. Set when
> - *              registering a device behind another one.
> - */
> -struct cros_ec_platform {
> -	const char *ec_name;
> -	u16 cmd_offset;
> -};
> -
> -struct cros_ec_debugfs;
>  
>  /**
>   * struct cros_ec_dev - ChromeOS EC device entry point.
> @@ -217,133 +40,4 @@ struct cros_ec_dev {
>  
>  #define to_cros_ec_dev(dev)  container_of(dev, struct cros_ec_dev, class_dev)
>  
> -/**
> - * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device.
> - * @ec_dev: Device to suspend.
> - *
> - * This can be called by drivers to handle a suspend event.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_suspend(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device.
> - * @ec_dev: Device to resume.
> - *
> - * This can be called by drivers to handle a resume event.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_resume(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_prepare_tx() - Prepare an outgoing message in the output buffer.
> - * @ec_dev: Device to register.
> - * @msg: Message to write.
> - *
> - * This is intended to be used by all ChromeOS EC drivers, but at present
> - * only SPI uses it. Once LPC uses the same protocol it can start using it.
> - * I2C could use it now, with a refactor of the existing code.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> -		       struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_check_result() - Check ec_msg->result.
> - * @ec_dev: EC device.
> - * @msg: Message to check.
> - *
> - * This is used by ChromeOS EC drivers to check the ec_msg->result for
> - * errors and to warn about them.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_check_result(struct cros_ec_device *ec_dev,
> -			 struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
> - * @ec_dev: EC device.
> - * @msg: Message to write.
> - *
> - * Call this to send a command to the ChromeOS EC.  This should be used
> - * instead of calling the EC's cmd_xfer() callback directly.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> -		     struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
> - * @ec_dev: EC device.
> - * @msg: Message to write.
> - *
> - * This function is identical to cros_ec_cmd_xfer, except it returns success
> - * status only if both the command was transmitted successfully and the EC
> - * replied with success status. It's not necessary to check msg->result when
> - * using this function.
> - *
> - * Return: The number of bytes transferred on success or negative error code.
> - */
> -int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> -			    struct cros_ec_command *msg);
> -
> -/**
> - * cros_ec_register() - Register a new ChromeOS EC, using the provided info.
> - * @ec_dev: Device to register.
> - *
> - * Before calling this, allocate a pointer to a new device and then fill
> - * in all the fields up to the --private-- marker.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_register(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_unregister() - Remove a ChromeOS EC.
> - * @ec_dev: Device to unregister.
> - *
> - * Call this to deregister a ChromeOS EC, then clean up any private data.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_unregister(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_query_all() -  Query the protocol version supported by the
> - *         ChromeOS EC.
> - * @ec_dev: Device to register.
> - *
> - * Return: 0 on success or negative error code.
> - */
> -int cros_ec_query_all(struct cros_ec_device *ec_dev);
> -
> -/**
> - * cros_ec_get_next_event() - Fetch next event from the ChromeOS EC.
> - * @ec_dev: Device to fetch event from.
> - * @wake_event: Pointer to a bool set to true upon return if the event might be
> - *              treated as a wake event. Ignored if null.
> - *
> - * Return: negative error code on errors; 0 for no data; or else number of
> - * bytes received (i.e., an event was retrieved successfully). Event types are
> - * written out to @ec_dev->event_data.event_type on success.
> - */
> -int cros_ec_get_next_event(struct cros_ec_device *ec_dev, bool *wake_event);
> -
> -/**
> - * cros_ec_get_host_event() - Return a mask of event set by the ChromeOS EC.
> - * @ec_dev: Device to fetch event from.
> - *
> - * When MKBP is supported, when the EC raises an interrupt, we collect the
> - * events raised and call the functions in the ec notifier. This function
> - * is a helper to know which events are raised.
> - *
> - * Return: 0 on error or non-zero bitmask of one or more EC_HOST_EVENT_*.
> - */
> -u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> -
>  #endif /* __LINUX_MFD_CROS_EC_H */
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> similarity index 100%
> rename from include/linux/mfd/cros_ec_commands.h
> rename to include/linux/platform_data/cros_ec_commands.h
> diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
> new file mode 100644
> index 000000000000..34dd9e5c1779
> --- /dev/null
> +++ b/include/linux/platform_data/cros_ec_proto.h
> @@ -0,0 +1,315 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * ChromeOS Embedded Controller protocol interface.
> + *
> + * Copyright (C) 2012 Google, Inc
> + */
> +
> +#ifndef __LINUX_CROS_EC_PROTO_H
> +#define __LINUX_CROS_EC_PROTO_H
> +
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +
> +#define CROS_EC_DEV_NAME	"cros_ec"
> +#define CROS_EC_DEV_FP_NAME	"cros_fp"
> +#define CROS_EC_DEV_ISH_NAME	"cros_ish"
> +#define CROS_EC_DEV_PD_NAME	"cros_pd"
> +#define CROS_EC_DEV_TP_NAME	"cros_tp"
> +
> +/*
> + * The EC is unresponsive for a time after a reboot command.  Add a
> + * simple delay to make sure that the bus stays locked.
> + */
> +#define EC_REBOOT_DELAY_MS		50
> +
> +/*
> + * Max bus-specific overhead incurred by request/responses.
> + * I2C requires 1 additional byte for requests.
> + * I2C requires 2 additional bytes for responses.
> + * SPI requires up to 32 additional bytes for responses.
> + */
> +#define EC_PROTO_VERSION_UNKNOWN	0
> +#define EC_MAX_REQUEST_OVERHEAD		1
> +#define EC_MAX_RESPONSE_OVERHEAD	32
> +
> +/*
> + * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
> + */
> +enum {
> +	EC_MSG_TX_HEADER_BYTES	= 3,
> +	EC_MSG_TX_TRAILER_BYTES	= 1,
> +	EC_MSG_TX_PROTO_BYTES	= EC_MSG_TX_HEADER_BYTES +
> +				  EC_MSG_TX_TRAILER_BYTES,
> +	EC_MSG_RX_PROTO_BYTES	= 3,
> +
> +	/* Max length of messages for proto 2*/
> +	EC_PROTO2_MSG_BYTES	= EC_PROTO2_MAX_PARAM_SIZE +
> +				  EC_MSG_TX_PROTO_BYTES,
> +
> +	EC_MAX_MSG_BYTES	= 64 * 1024,
> +};
> +
> +/**
> + * struct cros_ec_command - Information about a ChromeOS EC command.
> + * @version: Command version number (often 0).
> + * @command: Command to send (EC_CMD_...).
> + * @outsize: Outgoing length in bytes.
> + * @insize: Max number of bytes to accept from the EC.
> + * @result: EC's response to the command (separate from communication failure).
> + * @data: Where to put the incoming data from EC and outgoing data to EC.
> + */
> +struct cros_ec_command {
> +	uint32_t version;
> +	uint32_t command;
> +	uint32_t outsize;
> +	uint32_t insize;
> +	uint32_t result;
> +	uint8_t data[0];
> +};
> +
> +/**
> + * struct cros_ec_device - Information about a ChromeOS EC device.
> + * @phys_name: Name of physical comms layer (e.g. 'i2c-4').
> + * @dev: Device pointer for physical comms device
> + * @was_wake_device: True if this device was set to wake the system from
> + *                   sleep at the last suspend.
> + * @cros_class: The class structure for this device.
> + * @cmd_readmem: Direct read of the EC memory-mapped region, if supported.
> + *     @offset: Is within EC_LPC_ADDR_MEMMAP region.
> + *     @bytes: Number of bytes to read. zero means "read a string" (including
> + *             the trailing '\0'). At most only EC_MEMMAP_SIZE bytes can be
> + *             read. Caller must ensure that the buffer is large enough for the
> + *             result when reading a string.
> + * @max_request: Max size of message requested.
> + * @max_response: Max size of message response.
> + * @max_passthru: Max sice of passthru message.
> + * @proto_version: The protocol version used for this device.
> + * @priv: Private data.
> + * @irq: Interrupt to use.
> + * @id: Device id.
> + * @din: Input buffer (for data from EC). This buffer will always be
> + *       dword-aligned and include enough space for up to 7 word-alignment
> + *       bytes also, so we can ensure that the body of the message is always
> + *       dword-aligned (64-bit). We use this alignment to keep ARM and x86
> + *       happy. Probably word alignment would be OK, there might be a small
> + *       performance advantage to using dword.
> + * @dout: Output buffer (for data to EC). This buffer will always be
> + *        dword-aligned and include enough space for up to 7 word-alignment
> + *        bytes also, so we can ensure that the body of the message is always
> + *        dword-aligned (64-bit). We use this alignment to keep ARM and x86
> + *        happy. Probably word alignment would be OK, there might be a small
> + *        performance advantage to using dword.
> + * @din_size: Size of din buffer to allocate (zero to use static din).
> + * @dout_size: Size of dout buffer to allocate (zero to use static dout).
> + * @wake_enabled: True if this device can wake the system from sleep.
> + * @suspended: True if this device had been suspended.
> + * @cmd_xfer: Send command to EC and get response.
> + *            Returns the number of bytes received if the communication
> + *            succeeded, but that doesn't mean the EC was happy with the
> + *            command. The caller should check msg.result for the EC's result
> + *            code.
> + * @pkt_xfer: Send packet to EC and get response.
> + * @lock: One transaction at a time.
> + * @mkbp_event_supported: True if this EC supports the MKBP event protocol.
> + * @host_sleep_v1: True if this EC supports the sleep v1 command.
> + * @event_notifier: Interrupt event notifier for transport devices.
> + * @event_data: Raw payload transferred with the MKBP event.
> + * @event_size: Size in bytes of the event data.
> + * @host_event_wake_mask: Mask of host events that cause wake from suspend.
> + * @ec: The platform_device used by the mfd driver to interface with the
> + *      main EC.
> + * @pd: The platform_device used by the mfd driver to interface with the
> + *      PD behind an EC.
> + */
> +struct cros_ec_device {
> +	/* These are used by other drivers that want to talk to the EC */
> +	const char *phys_name;
> +	struct device *dev;
> +	bool was_wake_device;
> +	struct class *cros_class;
> +	int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
> +			   unsigned int bytes, void *dest);
> +
> +	/* These are used to implement the platform-specific interface */
> +	u16 max_request;
> +	u16 max_response;
> +	u16 max_passthru;
> +	u16 proto_version;
> +	void *priv;
> +	int irq;
> +	u8 *din;
> +	u8 *dout;
> +	int din_size;
> +	int dout_size;
> +	bool wake_enabled;
> +	bool suspended;
> +	int (*cmd_xfer)(struct cros_ec_device *ec,
> +			struct cros_ec_command *msg);
> +	int (*pkt_xfer)(struct cros_ec_device *ec,
> +			struct cros_ec_command *msg);
> +	struct mutex lock;
> +	bool mkbp_event_supported;
> +	bool host_sleep_v1;
> +	struct blocking_notifier_head event_notifier;
> +
> +	struct ec_response_get_next_event_v1 event_data;
> +	int event_size;
> +	u32 host_event_wake_mask;
> +
> +	/* The platform devices used by the mfd driver */
> +	struct platform_device *ec;
> +	struct platform_device *pd;
> +};
> +
> +/**
> + * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information.
> + * @sensor_num: Id of the sensor, as reported by the EC.
> + */
> +struct cros_ec_sensor_platform {
> +	u8 sensor_num;
> +};
> +
> +/**
> + * struct cros_ec_platform - ChromeOS EC platform information.
> + * @ec_name: Name of EC device (e.g. 'cros-ec', 'cros-pd', ...)
> + *           used in /dev/ and sysfs.
> + * @cmd_offset: Offset to apply for each command. Set when
> + *              registering a device behind another one.
> + */
> +struct cros_ec_platform {
> +	const char *ec_name;
> +	u16 cmd_offset;
> +};
> +
> +/**
> + * cros_ec_suspend() - Handle a suspend operation for the ChromeOS EC device.
> + * @ec_dev: Device to suspend.
> + *
> + * This can be called by drivers to handle a suspend event.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_suspend(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_resume() - Handle a resume operation for the ChromeOS EC device.
> + * @ec_dev: Device to resume.
> + *
> + * This can be called by drivers to handle a resume event.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_resume(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_prepare_tx() - Prepare an outgoing message in the output buffer.
> + * @ec_dev: Device to register.
> + * @msg: Message to write.
> + *
> + * This is intended to be used by all ChromeOS EC drivers, but at present
> + * only SPI uses it. Once LPC uses the same protocol it can start using it.
> + * I2C could use it now, with a refactor of the existing code.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> +		       struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_check_result() - Check ec_msg->result.
> + * @ec_dev: EC device.
> + * @msg: Message to check.
> + *
> + * This is used by ChromeOS EC drivers to check the ec_msg->result for
> + * errors and to warn about them.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_check_result(struct cros_ec_device *ec_dev,
> +			 struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_cmd_xfer() - Send a command to the ChromeOS EC.
> + * @ec_dev: EC device.
> + * @msg: Message to write.
> + *
> + * Call this to send a command to the ChromeOS EC.  This should be used
> + * instead of calling the EC's cmd_xfer() callback directly.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
> +		     struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
> + * @ec_dev: EC device.
> + * @msg: Message to write.
> + *
> + * This function is identical to cros_ec_cmd_xfer, except it returns success
> + * status only if both the command was transmitted successfully and the EC
> + * replied with success status. It's not necessary to check msg->result when
> + * using this function.
> + *
> + * Return: The number of bytes transferred on success or negative error code.
> + */
> +int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
> +			    struct cros_ec_command *msg);
> +
> +/**
> + * cros_ec_register() - Register a new ChromeOS EC, using the provided info.
> + * @ec_dev: Device to register.
> + *
> + * Before calling this, allocate a pointer to a new device and then fill
> + * in all the fields up to the --private-- marker.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_register(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_unregister() - Remove a ChromeOS EC.
> + * @ec_dev: Device to unregister.
> + *
> + * Call this to deregister a ChromeOS EC, then clean up any private data.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_unregister(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_query_all() -  Query the protocol version supported by the
> + *         ChromeOS EC.
> + * @ec_dev: Device to register.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_ec_query_all(struct cros_ec_device *ec_dev);
> +
> +/**
> + * cros_ec_get_next_event() - Fetch next event from the ChromeOS EC.
> + * @ec_dev: Device to fetch event from.
> + * @wake_event: Pointer to a bool set to true upon return if the event might be
> + *              treated as a wake event. Ignored if null.
> + *
> + * Return: negative error code on errors; 0 for no data; or else number of
> + * bytes received (i.e., an event was retrieved successfully). Event types are
> + * written out to @ec_dev->event_data.event_type on success.
> + */
> +int cros_ec_get_next_event(struct cros_ec_device *ec_dev, bool *wake_event);
> +
> +/**
> + * cros_ec_get_host_event() - Return a mask of event set by the ChromeOS EC.
> + * @ec_dev: Device to fetch event from.
> + *
> + * When MKBP is supported, when the EC raises an interrupt, we collect the
> + * events raised and call the functions in the ec notifier. This function
> + * is a helper to know which events are raised.
> + *
> + * Return: 0 on error or non-zero bitmask of one or more EC_HOST_EVENT_*.
> + */
> +u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
> +
> +#endif /* __LINUX_CROS_EC_PROTO_H */
> diff --git a/sound/soc/codecs/cros_ec_codec.c b/sound/soc/codecs/cros_ec_codec.c
> index 87830ed5ebf4..79bb4081d3c2 100644
> --- a/sound/soc/codecs/cros_ec_codec.c
> +++ b/sound/soc/codecs/cros_ec_codec.c
> @@ -9,9 +9,9 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/kernel.h>
> -#include <linux/mfd/cros_ec.h>
> -#include <linux/mfd/cros_ec_commands.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
>  #include <linux/platform_device.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply

* RE: Get involved
From: Gote, Nitin R @ 2019-06-20 10:27 UTC (permalink / raw)
  To: Jann Horn; +Cc: Kees Cook, Kernel Hardening
In-Reply-To: <CAG48ez1fGhPmARDo9F_h=aX5G4eS8ti76678ynrEF0=mqXH02Q@mail.gmail.com>

Hi Jann,

Thank you for the input.
I have started working on these and I will send a patch for review soon.

Best Regards,
Nitin Gote. 

-----Original Message-----
From: Jann Horn [mailto:jannh@google.com] 
Sent: Tuesday, June 18, 2019 5:06 PM
To: Gote, Nitin R <nitin.r.gote@intel.com>
Cc: Kees Cook <keescook@chromium.org>; Kernel Hardening <kernel-hardening@lists.openwall.com>; Shyam Saini <mayhs11saini@gmail.com>
Subject: Re: Get involved

On Tue, Jun 18, 2019 at 1:20 PM Gote, Nitin R <nitin.r.gote@intel.com> wrote:
>
> Hi Kees,
>
> I would like to be involved on upstream on security related topics.
> I'm planning to work on below items from KSPP to do list:
>         1. deprecate strcpy() in favor of strscpy().
>         2. deprecate strlcpy() in favor of strscpy().
>         3. deprecate strncpy() in favor of strscpy(), strscpy_pad(), or str2mem_pad().
>
> I'm thinking of following approach for above items :
>
> Approach 1 : Do we need to blindly replace strcpy() or strlcpy() or strncpy() with strscpy() in entire linux kernel tree ?
>                  (This approach is time consuming as lots of changes 
> need to do in single patch or multiple patch)

Linus wrote at <https://lore.kernel.org/lkml/CA+55aFwHCPnPf_xs6GJu37UBvg_BSiFPH2uQps7qNNFV8Ej-SA@mail.gmail.com/>:

| I wrote a longish merge message about why - but it boils down to me 
| hating the mindless trivial conversion patches. Which were not in the 
| pull request, but I want to make it clear to everybody that I have 
| absolutely zero interest in seeing such patches. I want to encourage 
| judicious use of strscpy() in new code, or in code that gets modified 
| because it is buggy or is updated for other reasons (and thus thought 
| about and tested), but I am *not* going to accept patches that do mass 
| conversions of strlcpy or strncpy to the new interface.

From the "longish merge message" at
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=30c44659f4a3e7e1f9f47e895591b4b40bf62671>:

| Every time we introduce a new-and-improved interface, people start 
| doing these interminable series of trivial conversion patches.
|
| And every time that happens, somebody does some silly mistake, and the 
| conversion patch to the improved interface actually makes things worse.
| Because the patch is mindnumbing and trivial, nobody has the attention 
| span to look at it carefully, and it's usually done over large 
| swatches of source code which means that not every conversion gets tested.
|
| So I'm pulling the strscpy() support because it *is* a better interface.
| But I will refuse to pull mindless conversion patches.  Use this in 
| places where it makes sense, but don't do trivial patches to fix 
| things that aren't actually known to be broken.

Unless Linus changed his mind about that in the years since then, you probably don't want to spend your time writing a patch Linus doesn't want.

> Approach 2 : Do we need to implement script or some mechanism which checks for functions likes strcpy(), strlcpy() or strncpy() and
>                  throw some deprecate error, if these functions found and suggest to use strscpy() ?

It would probably make sense to add warnings for strlcpy() and
strncpy() in scripts/checkpatch.pl.

^ permalink raw reply

* [Qemu-devel] [PATCH v3 0/4] target/mips: Misc fixes and maintenance for 4.1
From: Aleksandar Markovic @ 2019-06-20 10:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: arikalo, amarkovic

From: Aleksandar Markovic <amarkovic@wavecomp.com>

This series contains miscelaneous fixes, improvements, and
maintainance items intended to be integrated into QEMU 4.1.
I will gradually add patches by the end of June 2019.

v2->v3:

  - amendeded two patches on cleaning checkpatch warnings

v1->v2:

  - added two patches on cleaning checkpatch warnings

Aleksandar Markovic (4):
  MAINTAINERS: Update file items for MIPS Malta board
  MAINTAINERS: Consolidate MIPS disassembler-related items
  target/mips: Fix some space checkpatch errors in translate.c
  target/mips: Fix if-else-switch-case arms checkpatch errors in
    translate.c

 MAINTAINERS             |   6 +-
 target/mips/translate.c | 445 +++++++++++++++++++++++++++---------------------
 2 files changed, 257 insertions(+), 194 deletions(-)

-- 
2.7.4



^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.