Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 5/8] soc: ux500: Switch to use DEVICE_ATTR_RO()
From: Linus Walleij @ 2020-05-25 11:46 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Greg Kroah-Hartman, Arnd Bergmann, linux-kernel@vger.kernel.org,
	Linux ARM
In-Reply-To: <20200523170859.50003-6-sudeep.holla@arm.com>

On Sat, May 23, 2020 at 7:09 PM Sudeep Holla <sudeep.holla@arm.com> wrote:

> Move device attributes to DEVICE_ATTR_RO() as that would make things
> a lot more "obvious" what is happening over the existing __ATTR usage.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

_______________________________________________
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 6/8] soc: ux500: Use custom soc attribute group instead of device_create_file
From: Linus Walleij @ 2020-05-25 11:46 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Greg Kroah-Hartman, Arnd Bergmann, linux-kernel@vger.kernel.org,
	Linux ARM
In-Reply-To: <20200523170859.50003-7-sudeep.holla@arm.com>

On Sat, May 23, 2020 at 7:09 PM Sudeep Holla <sudeep.holla@arm.com> wrote:

> Commit c31e73121f4c ("base: soc: Handle custom soc information sysfs
> entries") introduced custom soc attribute group in soc_device_attribute
> structure but there are no users treewide. While trying to understand
> the motivation and tried to use it, it was found lot of existing custom
> attributes can moved to use it instead of device_create_file.
>
> Though most of these never remove/cleanup the custom attribute as they
> never call soc_device_unregister, using these custom attribute group
> eliminate the need for any cleanup as the driver infrastructure will
> take care of that.
>
> Let us remove device_create_file and start using the custom attribute
> group in soc_device_attribute.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

_______________________________________________
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 5/5] crypto: stm32/crc: protect from concurrent accesses
From: Nicolas TOROMANOFF @ 2020-05-25 11:49 UTC (permalink / raw)
  To: Ard Biesheuvel, Eric Biggers
  Cc: Alexandre TORGUE, Linux Kernel Mailing List, David S . Miller,
	Linux Crypto Mailing List, Maxime Coquelin,
	linux-stm32@st-md-mailman.stormreply.com, Linux ARM, Herbert Xu
In-Reply-To: <CAMj1kXFwt6cs-MJhAeMRF4-yiddm=ezq=qvSjA_sRAX+_Gdqhw@mail.gmail.com>

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Monday, May 25, 2020 11:07 AM
> To: Nicolas TOROMANOFF <nicolas.toromanoff@st.com>; Eric Biggers
> <ebiggers@kernel.org>
> On Mon, 25 May 2020 at 11:01, Nicolas TOROMANOFF
> <nicolas.toromanoff@st.com> wrote:
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > > Sent: Monday, May 25, 2020 9:46 AM
> > > To: Nicolas TOROMANOFF <nicolas.toromanoff@st.com>
> > > Subject: Re: [PATCH 5/5] crypto: stm32/crc: protect from concurrent
> > > accesses
> > >
> > > On Mon, 25 May 2020 at 09:24, Nicolas TOROMANOFF
> > > <nicolas.toromanoff@st.com> wrote:
> > > >
> > > > Hello,
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > > > Sent: Friday, May 22, 2020 6:12 PM> On Tue, 12 May 2020 at
> > > > > 16:13, Nicolas Toromanoff <nicolas.toromanoff@st.com> wrote:
> > > > > >
> > > > > > Protect STM32 CRC device from concurrent accesses.
> > > > > >
> > > > > > As we create a spinlocked section that increase with buffer
> > > > > > size, we provide a module parameter to release the pressure by
> > > > > > splitting critical section in chunks.
> > > > > >
> > > > > > Size of each chunk is defined in burst_size module parameter.
> > > > > > By default burst_size=0, i.e. don't split incoming buffer.
> > > > > >
> > > > > > Signed-off-by: Nicolas Toromanoff <nicolas.toromanoff@st.com>
> > > > >
> > > > > Would you mind explaining the usage model here? It looks like
> > > > > you are sharing a CRC hardware accelerator with a synchronous
> > > > > interface between different users by using spinlocks? You are
> > > > > aware that this will tie up the waiting CPUs completely during
> > > > > this time, right? So it would be much better to use a mutex
> > > > > here. Or perhaps it would make more sense to fall back to a s/w
> > > > > based CRC routine if the h/w is tied up
> > > working for another task?
> > > >
> > > > I know mutex are more acceptable here, but shash _update() and
> > > > _init() may be call from any context, and so I cannot take a mutex.
> > > > And to protect my concurrent HW access I only though about spinlock.
> > > > Due to possible constraint on CPUs, I add a burst_size option to
> > > > force slitting long buffer into smaller one, and so decrease time we take
> the lock.
> > > > But I didn't though to fallback to software CRC.
> > > >
> > > > I'll do a patch on top.
> > > > In in the burst_update() function I'll use a
> > > > spin_trylock_irqsave() and use
> > > software CRC32 if HW is already in use.
> > > >
> > >
> > > Right. I didn't even notice that you were keeping interrupts
> > > disabled the whole time when using the h/w block. That means that
> > > any serious use of this h/w block will make IRQ latency go through the roof.
> > >
> > > I recommend that you go back to the drawing board on this driver,
> > > rather than papering over the issues with a spin_trylock(). Perhaps
> > > it would be better to model it as a ahash (even though the h/w block
> > > itself is synchronous) and use a kthread to feed in the data.
> >
> > I thought when I updated the driver to move to a ahash interface, but
> > the main usage of crc32 is the ext4 fs, that calls the shash API.
> > Commit 877b5691f27a ("crypto: shash - remove shash_desc::flags")
> > removed possibility to sleep in shash callback. (before this commit
> > and with MAY_SLEEP option set, using a mutex may have been fine).
> >
> 
> According to that commit's log, sleeping is never fine for shash(), since it uses
> kmap_atomic() when iterating over the scatterlist.

Today, we could avoid using kmap_atomic() in shash_ashash_*() APIs (the
ones that Walks through the scatterlist) by using the
crypto_ahash_walk_first() function to initialize the shash_ahash walker
(note that this function is never call in current kernel source [to remove ?]).
Then shash_ahash_*() functions will call ahash_*() function that use kmap()
if (walk->flags & CRYPTO_ALG_ASYNC) [flag set by crypto_ahash_walk_first()]
The last kmap_atomic() will be in the shash_ahash_digest() call in the
optimize branch (that should be replaced by the no atomic one)

I didn't investigate more this way, because I didn't check the drawback of
using kmap() instead of kmap_atomic(), I wanted to avoid modifying behavior
of other drivers and using a function never use elsewhere in kernel scarred
me ;-).
If these updates correct visible bugs in the ahash usage of the stm32-crc32
code [no more "sleep while atomic" traces even with mutex in tests], 
Documentation states that shash API could be called from any context,
I cannot add mutex in them.

> > By now the solution I see is to use the spin_trylock_irqsave(),
> > fallback to software crc *AND* capping burst_size to ensure the locked
> section stay reasonable.
> >
> > Does this seems acceptable ?
> >
> 
> If the reason for disabling interrupts is to avoid deadlocks, wouldn't the switch
> to trylock() with a software fallback allow us to keep interrupts enabled?

Right, with the trylock, I don't see why we may need to mask interrupts.


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

^ permalink raw reply

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

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

The code above calls dw9768_runtime_resume() if
!pm_runtime_enabled(dev), but the clean-up code below the
entity_cleanup label doesn't have the corresponding
dw9768_runtime_suspend() call.

Best regards,
Tomasz

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

^ permalink raw reply

* [PATCH v2 2/3] iio: adc: at91-sama5d2_adc: remove predisable/postenable hooks
From: Alexandru Ardelean @ 2020-05-25 11:38 UTC (permalink / raw)
  To: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32
  Cc: alexandre.torgue, linus.walleij, songqiang1304521,
	mcoquelin.stm32, lorenzo.bianconi83, Alexandru Ardelean, shawnguo,
	s.hauer, jic23
In-Reply-To: <20200525113855.178821-1-alexandru.ardelean@analog.com>

This should be squashed into the first patch, but it's the more peculiar of
the changes.
I am not sure whether this is correct. The touchscreen channels shouldn't
be enabled by the IIO framework. So, we may need a different way to handle
those if needed.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/adc/at91-sama5d2_adc.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 9abbbdcc7420..f71071096392 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -937,14 +937,6 @@ static int at91_adc_buffer_preenable(struct iio_dev *indio_dev)
 	return 0;
 }
 
-static int at91_adc_buffer_postenable(struct iio_dev *indio_dev)
-{
-	if (at91_adc_current_chan_is_touch(indio_dev))
-		return 0;
-
-	return iio_triggered_buffer_postenable(indio_dev);
-}
-
 static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
 {
 	struct at91_adc_state *st = iio_priv(indio_dev);
@@ -995,19 +987,9 @@ static int at91_adc_buffer_postdisable(struct iio_dev *indio_dev)
 	return 0;
 }
 
-static int at91_adc_buffer_predisable(struct iio_dev *indio_dev)
-{
-	if (at91_adc_current_chan_is_touch(indio_dev))
-		return 0;
-
-	return iio_triggered_buffer_predisable(indio_dev);
-}
-
 static const struct iio_buffer_setup_ops at91_buffer_setup_ops = {
 	.preenable = &at91_adc_buffer_preenable,
 	.postdisable = &at91_adc_buffer_postdisable,
-	.postenable = &at91_adc_buffer_postenable,
-	.predisable = &at91_adc_buffer_predisable,
 };
 
 static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *indio,
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v2 1/3] iio: Move attach/detach of the poll func to the core
From: Alexandru Ardelean @ 2020-05-25 11:38 UTC (permalink / raw)
  To: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32
  Cc: Lars-Peter Clausen, alexandre.torgue, linus.walleij,
	songqiang1304521, mcoquelin.stm32, lorenzo.bianconi83,
	Alexandru Ardelean, shawnguo, s.hauer, jic23

From: Lars-Peter Clausen <lars@metafoo.de>

All devices using a triggered buffer need to attach and detach the trigger
to the device in order to properly work. Instead of doing this in each and
every driver by hand move this into the core.

At this point in time, all drivers should have been resolved to
attach/detach the poll-function in the same order.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changelog v1 -> v2:
- drivers/iio/accel/st_accel_buffer.c - remove err2 var and return directly
- drivers/iio/gyro/st_gyro_buffer.c - return earlier if
  st_sensors_set_enable() failed
- drivers/iio/light/vcnl4000.c - return directly on error in
  vcnl4010_buffer_postenable() & vcnl4010_buffer_predisable()
- drivers/iio/pressure/zpa2326.c - print error message on error paths only

 .../buffer/industrialio-triggered-buffer.c    | 10 +--------
 drivers/iio/iio_core_trigger.h                | 17 ++++++++++++++
 drivers/iio/industrialio-buffer.c             | 13 +++++++++++
 drivers/iio/industrialio-trigger.c            | 22 ++++---------------
 include/linux/iio/trigger_consumer.h          |  7 ------
 5 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/drivers/iio/buffer/industrialio-triggered-buffer.c b/drivers/iio/buffer/industrialio-triggered-buffer.c
index e8046c1ecd6b..6c20a83f887e 100644
--- a/drivers/iio/buffer/industrialio-triggered-buffer.c
+++ b/drivers/iio/buffer/industrialio-triggered-buffer.c
@@ -13,11 +13,6 @@
 #include <linux/iio/triggered_buffer.h>
 #include <linux/iio/trigger_consumer.h>
 
-static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
-	.postenable = &iio_triggered_buffer_postenable,
-	.predisable = &iio_triggered_buffer_predisable,
-};
-
 /**
  * iio_triggered_buffer_setup() - Setup triggered buffer and pollfunc
  * @indio_dev:		IIO device structure
@@ -67,10 +62,7 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
 	}
 
 	/* Ring buffer functions - here trigger setup related */
-	if (setup_ops)
-		indio_dev->setup_ops = setup_ops;
-	else
-		indio_dev->setup_ops = &iio_triggered_buffer_setup_ops;
+	indio_dev->setup_ops = setup_ops;
 
 	/* Flag that polled ring buffering is possible */
 	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
diff --git a/drivers/iio/iio_core_trigger.h b/drivers/iio/iio_core_trigger.h
index e59fe2f36bbb..9d1a92cc6480 100644
--- a/drivers/iio/iio_core_trigger.h
+++ b/drivers/iio/iio_core_trigger.h
@@ -18,6 +18,12 @@ void iio_device_register_trigger_consumer(struct iio_dev *indio_dev);
  **/
 void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev);
 
+
+int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+				 struct iio_poll_func *pf);
+int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+				 struct iio_poll_func *pf);
+
 #else
 
 /**
@@ -37,4 +43,15 @@ static void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
 {
 }
 
+static inline int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+					       struct iio_poll_func *pf)
+{
+	return 0;
+}
+static inline int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+					       struct iio_poll_func *pf)
+{
+	return 0;
+}
+
 #endif /* CONFIG_TRIGGER_CONSUMER */
diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 9fa238c0a7d4..329dd4d6757a 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -20,6 +20,7 @@
 
 #include <linux/iio/iio.h>
 #include "iio_core.h"
+#include "iio_core_trigger.h"
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/buffer_impl.h>
@@ -972,6 +973,13 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
 		}
 	}
 
+	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
+		ret = iio_trigger_attach_poll_func(indio_dev->trig,
+						   indio_dev->pollfunc);
+		if (ret)
+			goto err_disable_buffers;
+	}
+
 	return 0;
 
 err_disable_buffers:
@@ -998,6 +1006,11 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
 	if (list_empty(&indio_dev->buffer_list))
 		return 0;
 
+	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
+		iio_trigger_detach_poll_func(indio_dev->trig,
+					     indio_dev->pollfunc);
+	}
+
 	/*
 	 * If things go wrong at some step in disable we still need to continue
 	 * to perform the other steps, otherwise we leave the device in a
diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
index 53d1931f6be8..6f16357fd732 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -239,8 +239,8 @@ static void iio_trigger_put_irq(struct iio_trigger *trig, int irq)
  * the relevant function is in there may be the best option.
  */
 /* Worth protecting against double additions? */
-static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
-					struct iio_poll_func *pf)
+int iio_trigger_attach_poll_func(struct iio_trigger *trig,
+				 struct iio_poll_func *pf)
 {
 	int ret = 0;
 	bool notinuse
@@ -290,8 +290,8 @@ static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
 	return ret;
 }
 
-static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
-					 struct iio_poll_func *pf)
+int iio_trigger_detach_poll_func(struct iio_trigger *trig,
+				 struct iio_poll_func *pf)
 {
 	int ret = 0;
 	bool no_other_users
@@ -705,17 +705,3 @@ void iio_device_unregister_trigger_consumer(struct iio_dev *indio_dev)
 	if (indio_dev->trig)
 		iio_trigger_put(indio_dev->trig);
 }
-
-int iio_triggered_buffer_postenable(struct iio_dev *indio_dev)
-{
-	return iio_trigger_attach_poll_func(indio_dev->trig,
-					    indio_dev->pollfunc);
-}
-EXPORT_SYMBOL(iio_triggered_buffer_postenable);
-
-int iio_triggered_buffer_predisable(struct iio_dev *indio_dev)
-{
-	return iio_trigger_detach_poll_func(indio_dev->trig,
-					     indio_dev->pollfunc);
-}
-EXPORT_SYMBOL(iio_triggered_buffer_predisable);
diff --git a/include/linux/iio/trigger_consumer.h b/include/linux/iio/trigger_consumer.h
index c3c6ba5ec423..3aa2f132dd67 100644
--- a/include/linux/iio/trigger_consumer.h
+++ b/include/linux/iio/trigger_consumer.h
@@ -50,11 +50,4 @@ irqreturn_t iio_pollfunc_store_time(int irq, void *p);
 
 void iio_trigger_notify_done(struct iio_trigger *trig);
 
-/*
- * Two functions for common case where all that happens is a pollfunc
- * is attached and detached from a trigger
- */
-int iio_triggered_buffer_postenable(struct iio_dev *indio_dev);
-int iio_triggered_buffer_predisable(struct iio_dev *indio_dev);
-
 #endif
-- 
2.25.1


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

^ permalink raw reply related

* [PATCH v2 3/3] iio: remove iio_triggered_buffer_postenable()/iio_triggered_buffer_predisable()
From: Alexandru Ardelean @ 2020-05-25 11:38 UTC (permalink / raw)
  To: linux-iio, linux-kernel, linux-arm-kernel, linux-stm32
  Cc: Lars-Peter Clausen, alexandre.torgue, linus.walleij,
	songqiang1304521, mcoquelin.stm32, lorenzo.bianconi83,
	Alexandru Ardelean, shawnguo, s.hauer, jic23
In-Reply-To: <20200525113855.178821-1-alexandru.ardelean@analog.com>

From: Lars-Peter Clausen <lars@metafoo.de>

This patch should be squashed into the first one, as the first one is
breaking the build (intentionally) to make the IIO core files easier to
review.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/iio/accel/adxl372.c                 | 20 +++--------
 drivers/iio/accel/bmc150-accel-core.c       |  4 +--
 drivers/iio/accel/kxcjk-1013.c              |  2 --
 drivers/iio/accel/kxsd9.c                   |  2 --
 drivers/iio/accel/st_accel_buffer.c         | 22 +++---------
 drivers/iio/accel/stk8312.c                 |  2 --
 drivers/iio/accel/stk8ba50.c                |  2 --
 drivers/iio/adc/ad7266.c                    |  2 --
 drivers/iio/adc/ad7606.c                    |  3 +-
 drivers/iio/adc/ad7766.c                    |  2 --
 drivers/iio/adc/ad7768-1.c                  |  8 +----
 drivers/iio/adc/ad7887.c                    |  2 --
 drivers/iio/adc/ad_sigma_delta.c            |  5 ---
 drivers/iio/adc/dln2-adc.c                  | 12 +------
 drivers/iio/adc/mxs-lradc-adc.c             |  2 --
 drivers/iio/adc/stm32-adc.c                 | 36 +++----------------
 drivers/iio/adc/stm32-dfsdm-adc.c           | 39 +++------------------
 drivers/iio/adc/ti-adc084s021.c             |  2 --
 drivers/iio/adc/ti-ads1015.c                |  2 --
 drivers/iio/adc/vf610_adc.c                 |  7 +---
 drivers/iio/adc/xilinx-xadc-core.c          |  2 --
 drivers/iio/chemical/atlas-sensor.c         |  6 +---
 drivers/iio/dummy/iio_simple_dummy_buffer.c | 14 --------
 drivers/iio/gyro/bmg160_core.c              |  2 --
 drivers/iio/gyro/mpu3050-core.c             |  2 --
 drivers/iio/gyro/st_gyro_buffer.c           | 21 +++--------
 drivers/iio/humidity/hdc100x.c              | 12 +------
 drivers/iio/humidity/hts221_buffer.c        |  2 --
 drivers/iio/light/gp2ap020a00f.c            | 10 ------
 drivers/iio/light/isl29125.c                | 20 ++---------
 drivers/iio/light/rpr0521.c                 |  2 --
 drivers/iio/light/si1145.c                  |  2 --
 drivers/iio/light/st_uvis25_core.c          |  2 --
 drivers/iio/light/tcs3414.c                 | 20 ++---------
 drivers/iio/light/vcnl4000.c                | 35 ++++--------------
 drivers/iio/magnetometer/bmc150_magn.c      |  2 --
 drivers/iio/magnetometer/rm3100-core.c      |  2 --
 drivers/iio/magnetometer/st_magn_buffer.c   | 26 ++------------
 drivers/iio/potentiostat/lmp91000.c         | 13 ++-----
 drivers/iio/pressure/st_pressure_buffer.c   | 26 ++------------
 drivers/iio/pressure/zpa2326.c              | 27 ++++++--------
 drivers/iio/proximity/sx9310.c              |  2 --
 drivers/iio/proximity/sx9500.c              |  9 -----
 43 files changed, 58 insertions(+), 377 deletions(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index 60daf04ce188..53697ca09a22 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -795,13 +795,9 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
 	unsigned int mask;
 	int i, ret;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret < 0)
-		return ret;
-
 	ret = adxl372_set_interrupts(st, ADXL372_INT1_MAP_FIFO_FULL_MSK, 0);
 	if (ret < 0)
-		goto err;
+		return ret;
 
 	mask = *indio_dev->active_scan_mask;
 
@@ -810,10 +806,8 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
 			break;
 	}
 
-	if (i == ARRAY_SIZE(adxl372_axis_lookup_table)) {
-		ret = -EINVAL;
-		goto err;
-	}
+	if (i == ARRAY_SIZE(adxl372_axis_lookup_table))
+		return -EINVAL;
 
 	st->fifo_format = adxl372_axis_lookup_table[i].fifo_format;
 	st->fifo_set_size = bitmap_weight(indio_dev->active_scan_mask,
@@ -833,14 +827,10 @@ static int adxl372_buffer_postenable(struct iio_dev *indio_dev)
 	if (ret < 0) {
 		st->fifo_mode = ADXL372_FIFO_BYPASSED;
 		adxl372_set_interrupts(st, 0, 0);
-		goto err;
+		return ret;
 	}
 
 	return 0;
-
-err:
-	iio_triggered_buffer_predisable(indio_dev);
-	return ret;
 }
 
 static int adxl372_buffer_predisable(struct iio_dev *indio_dev)
@@ -851,7 +841,7 @@ static int adxl372_buffer_predisable(struct iio_dev *indio_dev)
 	st->fifo_mode = ADXL372_FIFO_BYPASSED;
 	adxl372_configure_fifo(st);
 
-	return iio_triggered_buffer_predisable(indio_dev);
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops adxl372_buffer_ops = {
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 121b4e89f038..a2ff7033202d 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -1411,7 +1411,7 @@ static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
 	int ret = 0;
 
 	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
-		return iio_triggered_buffer_postenable(indio_dev);
+		return 0;
 
 	mutex_lock(&data->mutex);
 
@@ -1443,7 +1443,7 @@ static int bmc150_accel_buffer_predisable(struct iio_dev *indio_dev)
 	struct bmc150_accel_data *data = iio_priv(indio_dev);
 
 	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
-		return iio_triggered_buffer_predisable(indio_dev);
+		return 0;
 
 	mutex_lock(&data->mutex);
 
diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index c9924a65c32a..209b0e189fd4 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -1027,9 +1027,7 @@ static const struct iio_chan_spec kxcjk1013_channels[] = {
 
 static const struct iio_buffer_setup_ops kxcjk1013_buffer_setup_ops = {
 	.preenable		= kxcjk1013_buffer_preenable,
-	.postenable		= iio_triggered_buffer_postenable,
 	.postdisable		= kxcjk1013_buffer_postdisable,
-	.predisable		= iio_triggered_buffer_predisable,
 };
 
 static const struct iio_info kxcjk1013_info = {
diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 0b876b2dc5bd..39da8a090822 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -252,8 +252,6 @@ static int kxsd9_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops kxsd9_buffer_setup_ops = {
 	.preenable = kxsd9_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = kxsd9_buffer_postdisable,
 };
 
diff --git a/drivers/iio/accel/st_accel_buffer.c b/drivers/iio/accel/st_accel_buffer.c
index b5c814ef1637..492263589e04 100644
--- a/drivers/iio/accel/st_accel_buffer.c
+++ b/drivers/iio/accel/st_accel_buffer.c
@@ -33,13 +33,9 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
 {
 	int err;
 
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err < 0)
-		return err;
-
 	err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
 	if (err < 0)
-		goto st_accel_buffer_predisable;
+		return err;
 
 	err = st_sensors_set_enable(indio_dev, true);
 	if (err < 0)
@@ -49,27 +45,19 @@ static int st_accel_buffer_postenable(struct iio_dev *indio_dev)
 
 st_accel_buffer_enable_all_axis:
 	st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
-st_accel_buffer_predisable:
-	iio_triggered_buffer_predisable(indio_dev);
 	return err;
 }
 
 static int st_accel_buffer_predisable(struct iio_dev *indio_dev)
 {
-	int err, err2;
+	int err;
 
 	err = st_sensors_set_enable(indio_dev, false);
 	if (err < 0)
-		goto st_accel_buffer_predisable;
-
-	err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
-
-st_accel_buffer_predisable:
-	err2 = iio_triggered_buffer_predisable(indio_dev);
-	if (!err)
-		err = err2;
+		return err;
 
-	return err;
+	return st_sensors_set_axis_enable(indio_dev,
+					  ST_SENSORS_ENABLE_ALL_AXIS);
 }
 
 static const struct iio_buffer_setup_ops st_accel_buffer_setup_ops = {
diff --git a/drivers/iio/accel/stk8312.c b/drivers/iio/accel/stk8312.c
index 58c160ccdee7..a7684545f12a 100644
--- a/drivers/iio/accel/stk8312.c
+++ b/drivers/iio/accel/stk8312.c
@@ -492,8 +492,6 @@ static int stk8312_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops stk8312_buffer_setup_ops = {
 	.preenable   = stk8312_buffer_preenable,
-	.postenable  = iio_triggered_buffer_postenable,
-	.predisable  = iio_triggered_buffer_predisable,
 	.postdisable = stk8312_buffer_postdisable,
 };
 
diff --git a/drivers/iio/accel/stk8ba50.c b/drivers/iio/accel/stk8ba50.c
index c70ddec29eb4..1eec8dfceefe 100644
--- a/drivers/iio/accel/stk8ba50.c
+++ b/drivers/iio/accel/stk8ba50.c
@@ -376,8 +376,6 @@ static int stk8ba50_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops stk8ba50_buffer_setup_ops = {
 	.preenable   = stk8ba50_buffer_preenable,
-	.postenable  = iio_triggered_buffer_postenable,
-	.predisable  = iio_triggered_buffer_predisable,
 	.postdisable = stk8ba50_buffer_postdisable,
 };
 
diff --git a/drivers/iio/adc/ad7266.c b/drivers/iio/adc/ad7266.c
index c8524f098883..d365c89d34ab 100644
--- a/drivers/iio/adc/ad7266.c
+++ b/drivers/iio/adc/ad7266.c
@@ -74,8 +74,6 @@ static int ad7266_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
 	.preenable = &ad7266_preenable,
-	.postenable = &iio_triggered_buffer_postenable,
-	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &ad7266_postdisable,
 };
 
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index e4683a68522a..a88e07869a69 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -499,7 +499,6 @@ static int ad7606_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct ad7606_state *st = iio_priv(indio_dev);
 
-	iio_triggered_buffer_postenable(indio_dev);
 	gpiod_set_value(st->gpio_convst, 1);
 
 	return 0;
@@ -511,7 +510,7 @@ static int ad7606_buffer_predisable(struct iio_dev *indio_dev)
 
 	gpiod_set_value(st->gpio_convst, 0);
 
-	return iio_triggered_buffer_predisable(indio_dev);
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops ad7606_buffer_ops = {
diff --git a/drivers/iio/adc/ad7766.c b/drivers/iio/adc/ad7766.c
index bc388ea41754..0b6d27cb5f81 100644
--- a/drivers/iio/adc/ad7766.c
+++ b/drivers/iio/adc/ad7766.c
@@ -178,8 +178,6 @@ static const struct ad7766_chip_info ad7766_chip_info[] = {
 
 static const struct iio_buffer_setup_ops ad7766_buffer_setup_ops = {
 	.preenable = &ad7766_preenable,
-	.postenable = &iio_triggered_buffer_postenable,
-	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &ad7766_postdisable,
 };
 
diff --git a/drivers/iio/adc/ad7768-1.c b/drivers/iio/adc/ad7768-1.c
index 0d132708c429..addd802ed5d7 100644
--- a/drivers/iio/adc/ad7768-1.c
+++ b/drivers/iio/adc/ad7768-1.c
@@ -490,7 +490,6 @@ static int ad7768_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
 
-	iio_triggered_buffer_postenable(indio_dev);
 	/*
 	 * Write a 1 to the LSB of the INTERFACE_FORMAT register to enter
 	 * continuous read mode. Subsequent data reads do not require an
@@ -502,17 +501,12 @@ static int ad7768_buffer_postenable(struct iio_dev *indio_dev)
 static int ad7768_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct ad7768_state *st = iio_priv(indio_dev);
-	int ret;
 
 	/*
 	 * To exit continuous read mode, perform a single read of the ADC_DATA
 	 * reg (0x2C), which allows further configuration of the device.
 	 */
-	ret = ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
-	if (ret < 0)
-		return ret;
-
-	return iio_triggered_buffer_predisable(indio_dev);
+	return ad7768_spi_reg_read(st, AD7768_REG_ADC_DATA, 3);
 }
 
 static const struct iio_buffer_setup_ops ad7768_buffer_ops = {
diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c
index c6a3428e950a..7ce665601ce4 100644
--- a/drivers/iio/adc/ad7887.c
+++ b/drivers/iio/adc/ad7887.c
@@ -136,8 +136,6 @@ static irqreturn_t ad7887_trigger_handler(int irq, void *p)
 
 static const struct iio_buffer_setup_ops ad7887_ring_setup_ops = {
 	.preenable = &ad7887_ring_preenable,
-	.postenable = &iio_triggered_buffer_postenable,
-	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &ad7887_ring_postdisable,
 };
 
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index dd3d54b3bc8b..3554ee6ee099 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -345,10 +345,6 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
 	unsigned int channel;
 	int ret;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret < 0)
-		return ret;
-
 	channel = find_first_bit(indio_dev->active_scan_mask,
 				 indio_dev->masklength);
 	ret = ad_sigma_delta_set_channel(sigma_delta,
@@ -441,7 +437,6 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p)
 
 static const struct iio_buffer_setup_ops ad_sd_buffer_setup_ops = {
 	.postenable = &ad_sd_buffer_postenable,
-	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &ad_sd_buffer_postdisable,
 	.validate_scan_mask = &iio_validate_scan_mask_onehot,
 };
diff --git a/drivers/iio/adc/dln2-adc.c b/drivers/iio/adc/dln2-adc.c
index 65c7c9329b1c..e4616b4a790a 100644
--- a/drivers/iio/adc/dln2-adc.c
+++ b/drivers/iio/adc/dln2-adc.c
@@ -524,10 +524,6 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
 	u16 conflict;
 	unsigned int trigger_chan;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret)
-		return ret;
-
 	mutex_lock(&dln2->mutex);
 
 	/* Enable ADC */
@@ -541,7 +537,6 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
 				(int)conflict);
 			ret = -EBUSY;
 		}
-		iio_triggered_buffer_predisable(indio_dev);
 		return ret;
 	}
 
@@ -555,7 +550,6 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
 		mutex_unlock(&dln2->mutex);
 		if (ret < 0) {
 			dev_dbg(&dln2->pdev->dev, "Problem in %s\n", __func__);
-			iio_triggered_buffer_predisable(indio_dev);
 			return ret;
 		}
 	} else {
@@ -568,7 +562,7 @@ static int dln2_adc_triggered_buffer_postenable(struct iio_dev *indio_dev)
 
 static int dln2_adc_triggered_buffer_predisable(struct iio_dev *indio_dev)
 {
-	int ret, ret2;
+	int ret;
 	struct dln2_adc *dln2 = iio_priv(indio_dev);
 
 	mutex_lock(&dln2->mutex);
@@ -586,10 +580,6 @@ static int dln2_adc_triggered_buffer_predisable(struct iio_dev *indio_dev)
 	if (ret < 0)
 		dev_dbg(&dln2->pdev->dev, "Problem in %s\n", __func__);
 
-	ret2 = iio_triggered_buffer_predisable(indio_dev);
-	if (ret == 0)
-		ret = ret2;
-
 	return ret;
 }
 
diff --git a/drivers/iio/adc/mxs-lradc-adc.c b/drivers/iio/adc/mxs-lradc-adc.c
index 9d2f74c2489a..7f49586944a9 100644
--- a/drivers/iio/adc/mxs-lradc-adc.c
+++ b/drivers/iio/adc/mxs-lradc-adc.c
@@ -568,8 +568,6 @@ static bool mxs_lradc_adc_validate_scan_mask(struct iio_dev *iio,
 
 static const struct iio_buffer_setup_ops mxs_lradc_adc_buffer_ops = {
 	.preenable = &mxs_lradc_adc_buffer_preenable,
-	.postenable = &iio_triggered_buffer_postenable,
-	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &mxs_lradc_adc_buffer_postdisable,
 	.validate_scan_mask = &mxs_lradc_adc_validate_scan_mask,
 };
diff --git a/drivers/iio/adc/stm32-adc.c b/drivers/iio/adc/stm32-adc.c
index ae622ee6d08c..cbc7f0467ae8 100644
--- a/drivers/iio/adc/stm32-adc.c
+++ b/drivers/iio/adc/stm32-adc.c
@@ -1482,7 +1482,7 @@ static int stm32_adc_dma_start(struct iio_dev *indio_dev)
 	return 0;
 }
 
-static int __stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
+static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	struct device *dev = indio_dev->dev.parent;
@@ -1527,22 +1527,7 @@ static int __stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
 	return ret;
 }
 
-static int stm32_adc_buffer_postenable(struct iio_dev *indio_dev)
-{
-	int ret;
-
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret < 0)
-		return ret;
-
-	ret = __stm32_adc_buffer_postenable(indio_dev);
-	if (ret < 0)
-		iio_triggered_buffer_predisable(indio_dev);
-
-	return ret;
-}
-
-static void __stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
+static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct stm32_adc *adc = iio_priv(indio_dev);
 	struct device *dev = indio_dev->dev.parent;
@@ -1561,19 +1546,8 @@ static void __stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
 
 	pm_runtime_mark_last_busy(dev);
 	pm_runtime_put_autosuspend(dev);
-}
-
-static int stm32_adc_buffer_predisable(struct iio_dev *indio_dev)
-{
-	int ret;
-
-	__stm32_adc_buffer_predisable(indio_dev);
-
-	ret = iio_triggered_buffer_predisable(indio_dev);
-	if (ret < 0)
-		dev_err(&indio_dev->dev, "predisable failed\n");
 
-	return ret;
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops stm32_adc_buffer_setup_ops = {
@@ -2016,7 +1990,7 @@ static int stm32_adc_suspend(struct device *dev)
 	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
 
 	if (iio_buffer_enabled(indio_dev))
-		__stm32_adc_buffer_predisable(indio_dev);
+		stm32_adc_buffer_predisable(indio_dev);
 
 	return pm_runtime_force_suspend(dev);
 }
@@ -2039,7 +2013,7 @@ static int stm32_adc_resume(struct device *dev)
 	if (ret < 0)
 		return ret;
 
-	return __stm32_adc_buffer_postenable(indio_dev);
+	return stm32_adc_buffer_postenable(indio_dev);
 }
 #endif
 
diff --git a/drivers/iio/adc/stm32-dfsdm-adc.c b/drivers/iio/adc/stm32-dfsdm-adc.c
index 76a60d93fe23..0f652165d781 100644
--- a/drivers/iio/adc/stm32-dfsdm-adc.c
+++ b/drivers/iio/adc/stm32-dfsdm-adc.c
@@ -993,7 +993,7 @@ static int stm32_dfsdm_update_scan_mode(struct iio_dev *indio_dev,
 	return 0;
 }
 
-static int __stm32_dfsdm_postenable(struct iio_dev *indio_dev)
+static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
 {
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
 	int ret;
@@ -1036,30 +1036,7 @@ static int __stm32_dfsdm_postenable(struct iio_dev *indio_dev)
 	return ret;
 }
 
-static int stm32_dfsdm_postenable(struct iio_dev *indio_dev)
-{
-	int ret;
-
-	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
-		ret = iio_triggered_buffer_postenable(indio_dev);
-		if (ret < 0)
-			return ret;
-	}
-
-	ret = __stm32_dfsdm_postenable(indio_dev);
-	if (ret < 0)
-		goto err_predisable;
-
-	return 0;
-
-err_predisable:
-	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
-		iio_triggered_buffer_predisable(indio_dev);
-
-	return ret;
-}
-
-static void __stm32_dfsdm_predisable(struct iio_dev *indio_dev)
+static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
 {
 	struct stm32_dfsdm_adc *adc = iio_priv(indio_dev);
 
@@ -1071,14 +1048,6 @@ static void __stm32_dfsdm_predisable(struct iio_dev *indio_dev)
 
 	if (adc->hwc)
 		iio_hw_consumer_disable(adc->hwc);
-}
-
-static int stm32_dfsdm_predisable(struct iio_dev *indio_dev)
-{
-	__stm32_dfsdm_predisable(indio_dev);
-
-	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED)
-		iio_triggered_buffer_predisable(indio_dev);
 
 	return 0;
 }
@@ -1667,7 +1636,7 @@ static int __maybe_unused stm32_dfsdm_adc_suspend(struct device *dev)
 	struct iio_dev *indio_dev = iio_priv_to_dev(adc);
 
 	if (iio_buffer_enabled(indio_dev))
-		__stm32_dfsdm_predisable(indio_dev);
+		stm32_dfsdm_predisable(indio_dev);
 
 	return 0;
 }
@@ -1690,7 +1659,7 @@ static int __maybe_unused stm32_dfsdm_adc_resume(struct device *dev)
 	}
 
 	if (iio_buffer_enabled(indio_dev))
-		__stm32_dfsdm_postenable(indio_dev);
+		stm32_dfsdm_postenable(indio_dev);
 
 	return 0;
 }
diff --git a/drivers/iio/adc/ti-adc084s021.c b/drivers/iio/adc/ti-adc084s021.c
index bdedf456ee05..75ec8a21d28e 100644
--- a/drivers/iio/adc/ti-adc084s021.c
+++ b/drivers/iio/adc/ti-adc084s021.c
@@ -187,8 +187,6 @@ static const struct iio_info adc084s021_info = {
 
 static const struct iio_buffer_setup_ops adc084s021_buffer_setup_ops = {
 	.preenable = adc084s021_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = adc084s021_buffer_postdisable,
 };
 
diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 5ea4f45d6bad..efb55c2bccf4 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -788,8 +788,6 @@ static int ads1015_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
 	.preenable	= ads1015_buffer_preenable,
-	.postenable	= iio_triggered_buffer_postenable,
-	.predisable	= iio_triggered_buffer_predisable,
 	.postdisable	= ads1015_buffer_postdisable,
 	.validate_scan_mask = &iio_validate_scan_mask_onehot,
 };
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index cb7380bf07ca..72b363ea4074 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -724,13 +724,8 @@ static int vf610_adc_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct vf610_adc *info = iio_priv(indio_dev);
 	unsigned int channel;
-	int ret;
 	int val;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret)
-		return ret;
-
 	val = readl(info->regs + VF610_REG_ADC_GC);
 	val |= VF610_ADC_ADCON;
 	writel(val, info->regs + VF610_REG_ADC_GC);
@@ -761,7 +756,7 @@ static int vf610_adc_buffer_predisable(struct iio_dev *indio_dev)
 
 	writel(hc_cfg, info->regs + VF610_REG_ADC_HC0);
 
-	return iio_triggered_buffer_predisable(indio_dev);
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops iio_triggered_buffer_setup_ops = {
diff --git a/drivers/iio/adc/xilinx-xadc-core.c b/drivers/iio/adc/xilinx-xadc-core.c
index d7fecab9252e..2e81aa390978 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -839,8 +839,6 @@ static int xadc_preenable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops xadc_buffer_ops = {
 	.preenable = &xadc_preenable,
-	.postenable = &iio_triggered_buffer_postenable,
-	.predisable = &iio_triggered_buffer_predisable,
 	.postdisable = &xadc_postdisable,
 };
 
diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
index a6d996ab9a66..362cf766a79a 100644
--- a/drivers/iio/chemical/atlas-sensor.c
+++ b/drivers/iio/chemical/atlas-sensor.c
@@ -398,10 +398,6 @@ static int atlas_buffer_postenable(struct iio_dev *indio_dev)
 	struct atlas_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret)
-		return ret;
-
 	ret = pm_runtime_get_sync(&data->client->dev);
 	if (ret < 0) {
 		pm_runtime_put_noidle(&data->client->dev);
@@ -425,7 +421,7 @@ static int atlas_buffer_predisable(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	return iio_triggered_buffer_predisable(indio_dev);
+	return 0;
 }
 
 static const struct iio_trigger_ops atlas_interrupt_trigger_ops = {
diff --git a/drivers/iio/dummy/iio_simple_dummy_buffer.c b/drivers/iio/dummy/iio_simple_dummy_buffer.c
index 17606eca42b4..8e13c53d4360 100644
--- a/drivers/iio/dummy/iio_simple_dummy_buffer.c
+++ b/drivers/iio/dummy/iio_simple_dummy_buffer.c
@@ -99,20 +99,6 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
 }
 
 static const struct iio_buffer_setup_ops iio_simple_dummy_buffer_setup_ops = {
-	/*
-	 * iio_triggered_buffer_postenable:
-	 * Generic function that simply attaches the pollfunc to the trigger.
-	 * Replace this to mess with hardware state before we attach the
-	 * trigger.
-	 */
-	.postenable = &iio_triggered_buffer_postenable,
-	/*
-	 * iio_triggered_buffer_predisable:
-	 * Generic function that simple detaches the pollfunc from the trigger.
-	 * Replace this to put hardware state back again after the trigger is
-	 * detached but before userspace knows we have disabled the ring.
-	 */
-	.predisable = &iio_triggered_buffer_predisable,
 };
 
 int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 428ddfc13acb..0d0bfdd6036e 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -1051,8 +1051,6 @@ static int bmg160_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops bmg160_buffer_setup_ops = {
 	.preenable = bmg160_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = bmg160_buffer_postdisable,
 };
 
diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index 8e908a749f95..de1cd17d57f0 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -662,8 +662,6 @@ static int mpu3050_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops mpu3050_buffer_setup_ops = {
 	.preenable = mpu3050_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = mpu3050_buffer_postdisable,
 };
 
diff --git a/drivers/iio/gyro/st_gyro_buffer.c b/drivers/iio/gyro/st_gyro_buffer.c
index 9c92ff7a82be..4feb7ada7195 100644
--- a/drivers/iio/gyro/st_gyro_buffer.c
+++ b/drivers/iio/gyro/st_gyro_buffer.c
@@ -33,13 +33,9 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
 {
 	int err;
 
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err < 0)
-		return err;
-
 	err = st_sensors_set_axis_enable(indio_dev, indio_dev->active_scan_mask[0]);
 	if (err < 0)
-		goto st_gyro_buffer_predisable;
+		return err;
 
 	err = st_sensors_set_enable(indio_dev, true);
 	if (err < 0)
@@ -49,27 +45,18 @@ static int st_gyro_buffer_postenable(struct iio_dev *indio_dev)
 
 st_gyro_buffer_enable_all_axis:
 	st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
-st_gyro_buffer_predisable:
-	iio_triggered_buffer_predisable(indio_dev);
 	return err;
 }
 
 static int st_gyro_buffer_predisable(struct iio_dev *indio_dev)
 {
-	int err, err2;
+	int err;
 
 	err = st_sensors_set_enable(indio_dev, false);
 	if (err < 0)
-		goto st_gyro_buffer_predisable;
-
-	err = st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
-
-st_gyro_buffer_predisable:
-	err2 = iio_triggered_buffer_predisable(indio_dev);
-	if (!err)
-		err = err2;
+		return err;
 
-	return err;
+	return st_sensors_set_axis_enable(indio_dev, ST_SENSORS_ENABLE_ALL_AXIS);
 }
 
 static const struct iio_buffer_setup_ops st_gyro_buffer_setup_ops = {
diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index 7ecd2ffa3132..9243f8546cfb 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -278,17 +278,11 @@ static int hdc100x_buffer_postenable(struct iio_dev *indio_dev)
 	struct hdc100x_data *data = iio_priv(indio_dev);
 	int ret;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret)
-		return ret;
-
 	/* Buffer is enabled. First set ACQ Mode, then attach poll func */
 	mutex_lock(&data->lock);
 	ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE,
 				    HDC100X_REG_CONFIG_ACQ_MODE);
 	mutex_unlock(&data->lock);
-	if (ret)
-		iio_triggered_buffer_predisable(indio_dev);
 
 	return ret;
 }
@@ -296,16 +290,12 @@ static int hdc100x_buffer_postenable(struct iio_dev *indio_dev)
 static int hdc100x_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct hdc100x_data *data = iio_priv(indio_dev);
-	int ret, ret2;
+	int ret;
 
 	mutex_lock(&data->lock);
 	ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_ACQ_MODE, 0);
 	mutex_unlock(&data->lock);
 
-	ret2 = iio_triggered_buffer_predisable(indio_dev);
-	if (ret == 0)
-		ret = ret2;
-
 	return ret;
 }
 
diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c
index 9fb3f33614d4..d18b39f30ea7 100644
--- a/drivers/iio/humidity/hts221_buffer.c
+++ b/drivers/iio/humidity/hts221_buffer.c
@@ -153,8 +153,6 @@ static int hts221_buffer_postdisable(struct iio_dev *iio_dev)
 
 static const struct iio_buffer_setup_ops hts221_buffer_ops = {
 	.preenable = hts221_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = hts221_buffer_postdisable,
 };
 
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 070d4cd0cf54..29d7af33efa1 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1390,12 +1390,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
 
 	mutex_lock(&data->lock);
 
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err < 0) {
-		mutex_unlock(&data->lock);
-		return err;
-	}
-
 	/*
 	 * Enable triggers according to the scan_mask. Enabling either
 	 * LIGHT_CLEAR or LIGHT_IR scan mode results in enabling ALS
@@ -1430,8 +1424,6 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
 		err = -ENOMEM;
 
 error_unlock:
-	if (err < 0)
-		iio_triggered_buffer_predisable(indio_dev);
 	mutex_unlock(&data->lock);
 
 	return err;
@@ -1465,8 +1457,6 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
 	if (err == 0)
 		kfree(data->buffer);
 
-	iio_triggered_buffer_predisable(indio_dev);
-
 	mutex_unlock(&data->lock);
 
 	return err;
diff --git a/drivers/iio/light/isl29125.c b/drivers/iio/light/isl29125.c
index 95611f5eff01..76065433a967 100644
--- a/drivers/iio/light/isl29125.c
+++ b/drivers/iio/light/isl29125.c
@@ -216,36 +216,20 @@ static const struct iio_info isl29125_info = {
 static int isl29125_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct isl29125_data *data = iio_priv(indio_dev);
-	int err;
-
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err)
-		return err;
 
 	data->conf1 |= ISL29125_MODE_RGB;
-	err = i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
+	return i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
 		data->conf1);
-	if (err) {
-		iio_triggered_buffer_predisable(indio_dev);
-		return err;
-	}
-
-	return 0;
 }
 
 static int isl29125_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct isl29125_data *data = iio_priv(indio_dev);
-	int ret;
 
 	data->conf1 &= ~ISL29125_MODE_MASK;
 	data->conf1 |= ISL29125_MODE_PD;
-	ret = i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
+	return i2c_smbus_write_byte_data(data->client, ISL29125_CONF1,
 		data->conf1);
-
-	iio_triggered_buffer_predisable(indio_dev);
-
-	return ret;
 }
 
 static const struct iio_buffer_setup_ops isl29125_buffer_setup_ops = {
diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
index a0a7aeae5a82..e43c050cc766 100644
--- a/drivers/iio/light/rpr0521.c
+++ b/drivers/iio/light/rpr0521.c
@@ -570,8 +570,6 @@ static int rpr0521_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops rpr0521_buffer_setup_ops = {
 	.preenable = rpr0521_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = rpr0521_buffer_postdisable,
 };
 
diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
index 0476c2bc8138..d409389ba38e 100644
--- a/drivers/iio/light/si1145.c
+++ b/drivers/iio/light/si1145.c
@@ -1171,8 +1171,6 @@ static bool si1145_validate_scan_mask(struct iio_dev *indio_dev,
 
 static const struct iio_buffer_setup_ops si1145_buffer_setup_ops = {
 	.preenable = si1145_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.validate_scan_mask = si1145_validate_scan_mask,
 };
 
diff --git a/drivers/iio/light/st_uvis25_core.c b/drivers/iio/light/st_uvis25_core.c
index d262c254b895..0951dbeb5d04 100644
--- a/drivers/iio/light/st_uvis25_core.c
+++ b/drivers/iio/light/st_uvis25_core.c
@@ -227,8 +227,6 @@ static int st_uvis25_buffer_postdisable(struct iio_dev *iio_dev)
 
 static const struct iio_buffer_setup_ops st_uvis25_buffer_ops = {
 	.preenable = st_uvis25_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = st_uvis25_buffer_postdisable,
 };
 
diff --git a/drivers/iio/light/tcs3414.c b/drivers/iio/light/tcs3414.c
index b542e5619ead..2dc3d5db5d08 100644
--- a/drivers/iio/light/tcs3414.c
+++ b/drivers/iio/light/tcs3414.c
@@ -243,35 +243,19 @@ static const struct iio_info tcs3414_info = {
 static int tcs3414_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct tcs3414_data *data = iio_priv(indio_dev);
-	int ret;
-
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret)
-		return ret;
 
 	data->control |= TCS3414_CONTROL_ADC_EN;
-	ret = i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
+	return i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
 		data->control);
-	if (ret)
-		iio_triggered_buffer_predisable(indio_dev);
-
-	return ret;
 }
 
 static int tcs3414_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct tcs3414_data *data = iio_priv(indio_dev);
-	int ret, ret2;
 
 	data->control &= ~TCS3414_CONTROL_ADC_EN;
-	ret = i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
+	return i2c_smbus_write_byte_data(data->client, TCS3414_CONTROL,
 		data->control);
-
-	ret2 = iio_triggered_buffer_predisable(indio_dev);
-	if (!ret)
-		ret = ret2;
-
-	return ret;
 }
 
 static const struct iio_buffer_setup_ops tcs3414_buffer_setup_ops = {
diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index 2a4b3d331055..5287ac7d8655 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -957,50 +957,29 @@ static int vcnl4010_buffer_postenable(struct iio_dev *indio_dev)
 	int ret;
 	int cmd;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret)
-		return ret;
-
 	/* Do not enable the buffer if we are already capturing events. */
-	if (vcnl4010_is_in_periodic_mode(data)) {
-		ret = -EBUSY;
-		goto end;
-	}
+	if (vcnl4010_is_in_periodic_mode(data))
+		return -EBUSY;
 
 	ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL,
 					VCNL4010_INT_PROX_EN);
 	if (ret < 0)
-		goto end;
+		return ret;
 
 	cmd = VCNL4000_SELF_TIMED_EN | VCNL4000_PROX_EN;
-	ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
-	if (ret < 0)
-		goto end;
-
-	return 0;
-end:
-	iio_triggered_buffer_predisable(indio_dev);
-
-	return ret;
+	return i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, cmd);
 }
 
 static int vcnl4010_buffer_predisable(struct iio_dev *indio_dev)
 {
 	struct vcnl4000_data *data = iio_priv(indio_dev);
-	int ret, ret_disable;
+	int ret;
 
 	ret = i2c_smbus_write_byte_data(data->client, VCNL4010_INT_CTRL, 0);
 	if (ret < 0)
-		goto end;
-
-	ret = i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
-
-end:
-	ret_disable = iio_triggered_buffer_predisable(indio_dev);
-	if (ret == 0)
-		ret = ret_disable;
+		return ret;
 
-	return ret;
+	return i2c_smbus_write_byte_data(data->client, VCNL4000_COMMAND, 0);
 }
 
 static const struct iio_buffer_setup_ops vcnl4010_buffer_ops = {
diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
index d4de16750b10..1422da8db803 100644
--- a/drivers/iio/magnetometer/bmc150_magn.c
+++ b/drivers/iio/magnetometer/bmc150_magn.c
@@ -836,8 +836,6 @@ static int bmc150_magn_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops bmc150_magn_buffer_setup_ops = {
 	.preenable = bmc150_magn_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = bmc150_magn_buffer_postdisable,
 };
 
diff --git a/drivers/iio/magnetometer/rm3100-core.c b/drivers/iio/magnetometer/rm3100-core.c
index 43a2e420c9c4..96ad92a3c5d0 100644
--- a/drivers/iio/magnetometer/rm3100-core.c
+++ b/drivers/iio/magnetometer/rm3100-core.c
@@ -463,8 +463,6 @@ static int rm3100_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops rm3100_buffer_ops = {
 	.preenable = rm3100_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = rm3100_buffer_postdisable,
 };
 
diff --git a/drivers/iio/magnetometer/st_magn_buffer.c b/drivers/iio/magnetometer/st_magn_buffer.c
index bb425c167a96..4917721fa2e5 100644
--- a/drivers/iio/magnetometer/st_magn_buffer.c
+++ b/drivers/iio/magnetometer/st_magn_buffer.c
@@ -31,34 +31,12 @@ int st_magn_trig_set_state(struct iio_trigger *trig, bool state)
 
 static int st_magn_buffer_postenable(struct iio_dev *indio_dev)
 {
-	int err;
-
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err < 0)
-		return err;
-
-	err = st_sensors_set_enable(indio_dev, true);
-	if (err < 0)
-		goto st_magn_buffer_predisable;
-
-	return 0;
-
-st_magn_buffer_predisable:
-	iio_triggered_buffer_predisable(indio_dev);
-	return err;
+	return st_sensors_set_enable(indio_dev, true);
 }
 
 static int st_magn_buffer_predisable(struct iio_dev *indio_dev)
 {
-	int err, err2;
-
-	err = st_sensors_set_enable(indio_dev, false);
-
-	err2 = iio_triggered_buffer_predisable(indio_dev);
-	if (!err)
-		err = err2;
-
-	return err;
+	return st_sensors_set_enable(indio_dev, false);
 }
 
 static const struct iio_buffer_setup_ops st_magn_buffer_setup_ops = {
diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index 2cb11da18e0f..5eac2dfae747 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -278,17 +278,8 @@ static const struct iio_trigger_ops lmp91000_trigger_ops = {
 static int lmp91000_buffer_postenable(struct iio_dev *indio_dev)
 {
 	struct lmp91000_data *data = iio_priv(indio_dev);
-	int err;
 
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err)
-		return err;
-
-	err = iio_channel_start_all_cb(data->cb_buffer);
-	if (err)
-		iio_triggered_buffer_predisable(indio_dev);
-
-	return err;
+	return iio_channel_start_all_cb(data->cb_buffer);
 }
 
 static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
@@ -297,7 +288,7 @@ static int lmp91000_buffer_predisable(struct iio_dev *indio_dev)
 
 	iio_channel_stop_all_cb(data->cb_buffer);
 
-	return iio_triggered_buffer_predisable(indio_dev);
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops lmp91000_buffer_setup_ops = {
diff --git a/drivers/iio/pressure/st_pressure_buffer.c b/drivers/iio/pressure/st_pressure_buffer.c
index 418dbf9e6e1e..7cf6f06797e1 100644
--- a/drivers/iio/pressure/st_pressure_buffer.c
+++ b/drivers/iio/pressure/st_pressure_buffer.c
@@ -31,34 +31,12 @@ int st_press_trig_set_state(struct iio_trigger *trig, bool state)
 
 static int st_press_buffer_postenable(struct iio_dev *indio_dev)
 {
-	int err;
-
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err < 0)
-		return err;
-
-	err = st_sensors_set_enable(indio_dev, true);
-	if (err < 0)
-		goto st_press_buffer_predisable;
-
-	return 0;
-
-st_press_buffer_predisable:
-	iio_triggered_buffer_predisable(indio_dev);
-	return err;
+	return st_sensors_set_enable(indio_dev, true);
 }
 
 static int st_press_buffer_predisable(struct iio_dev *indio_dev)
 {
-	int err, err2;
-
-	err = st_sensors_set_enable(indio_dev, false);
-
-	err2 = iio_triggered_buffer_predisable(indio_dev);
-	if (!err)
-		err = err2;
-
-	return err;
+	return st_sensors_set_enable(indio_dev, false);
 }
 
 static const struct iio_buffer_setup_ops st_press_buffer_setup_ops = {
diff --git a/drivers/iio/pressure/zpa2326.c b/drivers/iio/pressure/zpa2326.c
index 37fe851f89af..60948ad68cbd 100644
--- a/drivers/iio/pressure/zpa2326.c
+++ b/drivers/iio/pressure/zpa2326.c
@@ -1242,19 +1242,17 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
 	const struct zpa2326_private *priv = iio_priv(indio_dev);
 	int                           err;
 
-	/* Plug our own trigger event handler. */
-	err = iio_triggered_buffer_postenable(indio_dev);
-	if (err)
-		goto err;
-
 	if (!priv->waken) {
 		/*
 		 * We were already power supplied. Just clear hardware FIFO to
 		 * get rid of samples acquired during previous rounds (if any).
 		 */
 		err = zpa2326_clear_fifo(indio_dev, 0);
-		if (err)
-			goto err_buffer_predisable;
+		if (err) {
+			zpa2326_err(indio_dev,
+				    "failed to enable buffering (%d)", err);
+			return err;
+		}
 	}
 
 	if (!iio_trigger_using_own(indio_dev) && priv->waken) {
@@ -1263,18 +1261,14 @@ static int zpa2326_postenable_buffer(struct iio_dev *indio_dev)
 		 * powered up: reconfigure one-shot mode.
 		 */
 		err = zpa2326_config_oneshot(indio_dev, priv->irq);
-		if (err)
-			goto err_buffer_predisable;
+		if (err) {
+			zpa2326_err(indio_dev,
+				    "failed to enable buffering (%d)", err);
+			return err;
+		}
 	}
 
 	return 0;
-
-err_buffer_predisable:
-	iio_triggered_buffer_predisable(indio_dev);
-err:
-	zpa2326_err(indio_dev, "failed to enable buffering (%d)", err);
-
-	return err;
 }
 
 static int zpa2326_postdisable_buffer(struct iio_dev *indio_dev)
@@ -1287,7 +1281,6 @@ static int zpa2326_postdisable_buffer(struct iio_dev *indio_dev)
 static const struct iio_buffer_setup_ops zpa2326_buffer_setup_ops = {
 	.preenable   = zpa2326_preenable_buffer,
 	.postenable  = zpa2326_postenable_buffer,
-	.predisable  = iio_triggered_buffer_predisable,
 	.postdisable = zpa2326_postdisable_buffer
 };
 
diff --git a/drivers/iio/proximity/sx9310.c b/drivers/iio/proximity/sx9310.c
index d161f3061e35..764b147c8bde 100644
--- a/drivers/iio/proximity/sx9310.c
+++ b/drivers/iio/proximity/sx9310.c
@@ -736,8 +736,6 @@ static int sx9310_buffer_postdisable(struct iio_dev *indio_dev)
 
 static const struct iio_buffer_setup_ops sx9310_buffer_setup_ops = {
 	.preenable = sx9310_buffer_preenable,
-	.postenable = iio_triggered_buffer_postenable,
-	.predisable = iio_triggered_buffer_predisable,
 	.postdisable = sx9310_buffer_postdisable,
 };
 
diff --git a/drivers/iio/proximity/sx9500.c b/drivers/iio/proximity/sx9500.c
index 287d288e40c2..129c14e9a359 100644
--- a/drivers/iio/proximity/sx9500.c
+++ b/drivers/iio/proximity/sx9500.c
@@ -680,10 +680,6 @@ static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
 	struct sx9500_data *data = iio_priv(indio_dev);
 	int ret = 0, i;
 
-	ret = iio_triggered_buffer_postenable(indio_dev);
-	if (ret)
-		return ret;
-
 	mutex_lock(&data->mutex);
 
 	for (i = 0; i < SX9500_NUM_CHANNELS; i++)
@@ -700,9 +696,6 @@ static int sx9500_buffer_postenable(struct iio_dev *indio_dev)
 
 	mutex_unlock(&data->mutex);
 
-	if (ret)
-		iio_triggered_buffer_predisable(indio_dev);
-
 	return ret;
 }
 
@@ -727,8 +720,6 @@ static int sx9500_buffer_predisable(struct iio_dev *indio_dev)
 
 	mutex_unlock(&data->mutex);
 
-	iio_triggered_buffer_predisable(indio_dev);
-
 	return ret;
 }
 
-- 
2.25.1


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

^ permalink raw reply related

* Re: [PATCH 5/5] crypto: stm32/crc: protect from concurrent accesses
From: Ard Biesheuvel @ 2020-05-25 12:01 UTC (permalink / raw)
  To: Nicolas TOROMANOFF
  Cc: Alexandre TORGUE, Linux Kernel Mailing List,
	linux-stm32@st-md-mailman.stormreply.com, Eric Biggers,
	Linux Crypto Mailing List, Maxime Coquelin, David S . Miller,
	Linux ARM, Herbert Xu
In-Reply-To: <31e99726fa6544fcaac88490de3186e6@SFHDAG6NODE1.st.com>

On Mon, 25 May 2020 at 13:49, Nicolas TOROMANOFF
<nicolas.toromanoff@st.com> wrote:
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Sent: Monday, May 25, 2020 11:07 AM
> > To: Nicolas TOROMANOFF <nicolas.toromanoff@st.com>; Eric Biggers
> > <ebiggers@kernel.org>
> > On Mon, 25 May 2020 at 11:01, Nicolas TOROMANOFF
> > <nicolas.toromanoff@st.com> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > > Sent: Monday, May 25, 2020 9:46 AM
> > > > To: Nicolas TOROMANOFF <nicolas.toromanoff@st.com>
> > > > Subject: Re: [PATCH 5/5] crypto: stm32/crc: protect from concurrent
> > > > accesses
> > > >
> > > > On Mon, 25 May 2020 at 09:24, Nicolas TOROMANOFF
> > > > <nicolas.toromanoff@st.com> wrote:
> > > > >
> > > > > Hello,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > > > > Sent: Friday, May 22, 2020 6:12 PM> On Tue, 12 May 2020 at
> > > > > > 16:13, Nicolas Toromanoff <nicolas.toromanoff@st.com> wrote:
> > > > > > >
> > > > > > > Protect STM32 CRC device from concurrent accesses.
> > > > > > >
> > > > > > > As we create a spinlocked section that increase with buffer
> > > > > > > size, we provide a module parameter to release the pressure by
> > > > > > > splitting critical section in chunks.
> > > > > > >
> > > > > > > Size of each chunk is defined in burst_size module parameter.
> > > > > > > By default burst_size=0, i.e. don't split incoming buffer.
> > > > > > >
> > > > > > > Signed-off-by: Nicolas Toromanoff <nicolas.toromanoff@st.com>
> > > > > >
> > > > > > Would you mind explaining the usage model here? It looks like
> > > > > > you are sharing a CRC hardware accelerator with a synchronous
> > > > > > interface between different users by using spinlocks? You are
> > > > > > aware that this will tie up the waiting CPUs completely during
> > > > > > this time, right? So it would be much better to use a mutex
> > > > > > here. Or perhaps it would make more sense to fall back to a s/w
> > > > > > based CRC routine if the h/w is tied up
> > > > working for another task?
> > > > >
> > > > > I know mutex are more acceptable here, but shash _update() and
> > > > > _init() may be call from any context, and so I cannot take a mutex.
> > > > > And to protect my concurrent HW access I only though about spinlock.
> > > > > Due to possible constraint on CPUs, I add a burst_size option to
> > > > > force slitting long buffer into smaller one, and so decrease time we take
> > the lock.
> > > > > But I didn't though to fallback to software CRC.
> > > > >
> > > > > I'll do a patch on top.
> > > > > In in the burst_update() function I'll use a
> > > > > spin_trylock_irqsave() and use
> > > > software CRC32 if HW is already in use.
> > > > >
> > > >
> > > > Right. I didn't even notice that you were keeping interrupts
> > > > disabled the whole time when using the h/w block. That means that
> > > > any serious use of this h/w block will make IRQ latency go through the roof.
> > > >
> > > > I recommend that you go back to the drawing board on this driver,
> > > > rather than papering over the issues with a spin_trylock(). Perhaps
> > > > it would be better to model it as a ahash (even though the h/w block
> > > > itself is synchronous) and use a kthread to feed in the data.
> > >
> > > I thought when I updated the driver to move to a ahash interface, but
> > > the main usage of crc32 is the ext4 fs, that calls the shash API.
> > > Commit 877b5691f27a ("crypto: shash - remove shash_desc::flags")
> > > removed possibility to sleep in shash callback. (before this commit
> > > and with MAY_SLEEP option set, using a mutex may have been fine).
> > >
> >
> > According to that commit's log, sleeping is never fine for shash(), since it uses
> > kmap_atomic() when iterating over the scatterlist.
>
> Today, we could avoid using kmap_atomic() in shash_ashash_*() APIs (the
> ones that Walks through the scatterlist) by using the
> crypto_ahash_walk_first() function to initialize the shash_ahash walker
> (note that this function is never call in current kernel source [to remove ?]).
> Then shash_ahash_*() functions will call ahash_*() function that use kmap()
> if (walk->flags & CRYPTO_ALG_ASYNC) [flag set by crypto_ahash_walk_first()]
> The last kmap_atomic() will be in the shash_ahash_digest() call in the
> optimize branch (that should be replaced by the no atomic one)
>
> I didn't investigate more this way, because I didn't check the drawback of
> using kmap() instead of kmap_atomic(), I wanted to avoid modifying behavior
> of other drivers and using a function never use elsewhere in kernel scarred
> me ;-).
> If these updates correct visible bugs in the ahash usage of the stm32-crc32
> code [no more "sleep while atomic" traces even with mutex in tests],
> Documentation states that shash API could be called from any context,
> I cannot add mutex in them.
>
> > > By now the solution I see is to use the spin_trylock_irqsave(),
> > > fallback to software crc *AND* capping burst_size to ensure the locked
> > section stay reasonable.
> > >
> > > Does this seems acceptable ?
> > >
> >
> > If the reason for disabling interrupts is to avoid deadlocks, wouldn't the switch
> > to trylock() with a software fallback allow us to keep interrupts enabled?
>
> Right, with the trylock, I don't see why we may need to mask interrupts.
>
>

OK, then the fix should be easy.

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

^ permalink raw reply

* Re: [PATCH 0/3] STMFX power related fixes
From: Amelie DELAUNAY @ 2020-05-25 12:02 UTC (permalink / raw)
  To: Lee Jones, Maxime Coquelin, Alexandre Torgue
  Cc: linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <20200422090833.9743-1-amelie.delaunay@st.com>

Hi,

Gentle reminder regarding this series sent one month ago.

Regards,
Amelie

On 4/22/20 11:08 AM, Amelie Delaunay wrote:
> With suspend/resume tests on STM32MP157C-EV1 board, on which STMFX is used by
> several devices, some errors could occurred: -6 when trying to restore STMFX
> registers, spurious interrupts after disabling supply...
> This patchset fixes all these issues and cleans IRQ init error path.
> 
> Amelie Delaunay (3):
>    mfd: stmfx: reset chip on resume as supply was disabled
>    mfd: stmfx: fix stmfx_irq_init error path
>    mfd: stmfx: disable irq in suspend to avoid spurious interrupt
> 
>   drivers/mfd/stmfx.c       | 22 ++++++++++++++++++++--
>   include/linux/mfd/stmfx.h |  1 +
>   2 files changed, 21 insertions(+), 2 deletions(-)
> 

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

^ permalink raw reply

* Re: [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro
From: Thomas Zimmermann @ 2020-05-25 12:03 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: alexandre.belloni, linux-aspeed, narmstrong, airlied, liviu.dudau,
	dri-devel, paul, laurent.pinchart, mihail.atanassov, marex,
	khilman, abrodkin, kong.kongxinwei, xinliang.liu,
	ludovic.desroches, tomi.valkeinen, james.qian.wang, linux-imx,
	joel, alexandre.torgue, puck.chen, s.hauer, alison.wang, jsarha,
	wens, vincent.abriou, linux-arm-kernel, mcoquelin.stm32,
	bbrezillon, andrew, philippe.cornu, yannick.fertre,
	kieran.bingham+renesas, kernel, zourongrong, shawnguo
In-Reply-To: <20200522174835.GA1087580@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 12225 bytes --]

Hi Sam

Am 22.05.20 um 19:48 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Fri, May 22, 2020 at 03:52:26PM +0200, Thomas Zimmermann wrote:
>> Rename the macro to DRM_GEM_CMA_DRIVER_OPS to align with SHMEM
>> helpers.
> This part is fine, I like that the naming is somehow consistent.
> 
>> An internal version is provided for drivers that override
>> the default .dumb_create callback. Adapt drivers to the changes.
> I loathe anything named __foo or __FOO. This __ signals to me
> that the author was clueless in naming - or some sort.
> I know that __ is used in some lib headers - but thats not the case
> here.

I agree with your comment and I've been trying to find a better name
before posting the patchset. I considered something like
DRM_GEM_CMA_DRIVER_OPS_INTERNAL(), but wasn't happy with that either. In
the end, I uses the double underscore to push driver authors towards the
other macro. This one's only for the special case of settings a separate
implementation for .dumb_create().

> 
> But I love that we have a variant that takes a create function.
> So we do not have to escape from the nice macro.
> The macro is another way to tell me as rewiewer that this
> drivers uses all the default helpers for this.
> 
> 
> So critizising the name I better suggest something that
> I personally like better:
> 
> DRM_GEM_CMA_DRIVER_OPS_CREATE()

Can we at least use DRM_GEM_CMA_DRIVER_OPS_WITH_DUMB_CREATE ? Because
there's also gem_object_create.

I'll update the patches accordingly.

I noticed that most of the affected drivers do some kind of alignment
calculation in their dumb_create code. IMHO in the long run, we should
move such calculations into the default implementation and put the
control paramters into struct drm_mode_config.

Best regards
Thomas

> 
> It would look like this:
> 	/* GEM Operations */
> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
> -	.dumb_create            = drm_sun4i_gem_dumb_create,
> +	DRM_GEM_CMA_DRIVER_OPS_CREATE(drm_sun4i_gem_dumb_create),
> 
> 
> 
> Please fix zte/zx_drm_drv.c which also uses DRM_GEM_CMA_VMAP_DRIVER_OPS.
> 
> 
> The naming is a bikeshedding topic that we may not agree on, soo..
> 
> With zte fixed the patch is:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> 	Sam
> 
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/drm_gem_cma_helper.c |  2 +-
>>  drivers/gpu/drm/sun4i/sun4i_drv.c    |  3 +--
>>  drivers/gpu/drm/tidss/tidss_drv.c    |  2 +-
>>  drivers/gpu/drm/tiny/hx8357d.c       |  2 +-
>>  drivers/gpu/drm/tiny/ili9225.c       |  2 +-
>>  drivers/gpu/drm/tiny/ili9341.c       |  2 +-
>>  drivers/gpu/drm/tiny/ili9486.c       |  2 +-
>>  drivers/gpu/drm/tiny/mi0283qt.c      |  2 +-
>>  drivers/gpu/drm/tiny/repaper.c       |  2 +-
>>  drivers/gpu/drm/tiny/st7586.c        |  2 +-
>>  drivers/gpu/drm/tiny/st7735r.c       |  2 +-
>>  include/drm/drm_gem_cma_helper.h     | 24 ++++++++++++++++++++----
>>  12 files changed, 31 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
>> index 12e98fb28229d..6fa4d2f2e3987 100644
>> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
>> @@ -620,7 +620,7 @@ EXPORT_SYMBOL(drm_cma_gem_create_object_default_funcs);
>>   * address set. This address is released when the object is freed.
>>   *
>>   * This function can be used as the &drm_driver.gem_prime_import_sg_table
>> - * callback. The DRM_GEM_CMA_VMAP_DRIVER_OPS() macro provides a shortcut to set
>> + * callback. The &DRM_GEM_CMA_DRIVER_OPS macro provides a shortcut to set
>>   * the necessary DRM driver operations.
>>   *
>>   * Returns:
>> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> index 328272ff77d84..012855fd89c24 100644
>> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
>> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
>> @@ -52,8 +52,7 @@ static struct drm_driver sun4i_drv_driver = {
>>  	.minor			= 0,
>>  
>>  	/* GEM Operations */
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> -	.dumb_create		= drm_sun4i_gem_dumb_create,
>> +	__DRM_GEM_CMA_DRIVER_OPS(drm_sun4i_gem_dumb_create),
>>  };
>>  
>>  static int sun4i_drv_bind(struct device *dev)
>> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
>> index 99edc66ebdef2..1753cdc74ebda 100644
>> --- a/drivers/gpu/drm/tidss/tidss_drv.c
>> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
>> @@ -112,7 +112,7 @@ static struct drm_driver tidss_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &tidss_fops,
>>  	.release		= tidss_release,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.name			= "tidss",
>>  	.desc			= "TI Keystone DSS",
>>  	.date			= "20180215",
>> diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
>> index b4bc358a3269a..592da71d7ca70 100644
>> --- a/drivers/gpu/drm/tiny/hx8357d.c
>> +++ b/drivers/gpu/drm/tiny/hx8357d.c
>> @@ -196,7 +196,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
>>  static struct drm_driver hx8357d_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &hx8357d_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "hx8357d",
>>  	.desc			= "HX8357D",
>> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
>> index d1a5ab6747d5c..368ff6c8a1efb 100644
>> --- a/drivers/gpu/drm/tiny/ili9225.c
>> +++ b/drivers/gpu/drm/tiny/ili9225.c
>> @@ -346,7 +346,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
>>  static struct drm_driver ili9225_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &ili9225_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.name			= "ili9225",
>>  	.desc			= "Ilitek ILI9225",
>>  	.date			= "20171106",
>> diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
>> index bb819f45a5d3b..e1b9043ef7a0a 100644
>> --- a/drivers/gpu/drm/tiny/ili9341.c
>> +++ b/drivers/gpu/drm/tiny/ili9341.c
>> @@ -152,7 +152,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
>>  static struct drm_driver ili9341_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &ili9341_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "ili9341",
>>  	.desc			= "Ilitek ILI9341",
>> diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
>> index 2702ea557d297..90a17f40fdf0c 100644
>> --- a/drivers/gpu/drm/tiny/ili9486.c
>> +++ b/drivers/gpu/drm/tiny/ili9486.c
>> @@ -165,7 +165,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9486_fops);
>>  static struct drm_driver ili9486_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &ili9486_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "ili9486",
>>  	.desc			= "Ilitek ILI9486",
>> diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
>> index 08ac549ab0f7f..6624c2098fba2 100644
>> --- a/drivers/gpu/drm/tiny/mi0283qt.c
>> +++ b/drivers/gpu/drm/tiny/mi0283qt.c
>> @@ -156,7 +156,7 @@ DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops);
>>  static struct drm_driver mi0283qt_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &mi0283qt_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "mi0283qt",
>>  	.desc			= "Multi-Inno MI0283QT",
>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
>> index 1c0e7169545b4..877dcece25828 100644
>> --- a/drivers/gpu/drm/tiny/repaper.c
>> +++ b/drivers/gpu/drm/tiny/repaper.c
>> @@ -946,7 +946,7 @@ DEFINE_DRM_GEM_CMA_FOPS(repaper_fops);
>>  static struct drm_driver repaper_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &repaper_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.name			= "repaper",
>>  	.desc			= "Pervasive Displays RePaper e-ink panels",
>>  	.date			= "20170405",
>> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
>> index 2a1fae422f7a2..ec84bdc51f60d 100644
>> --- a/drivers/gpu/drm/tiny/st7586.c
>> +++ b/drivers/gpu/drm/tiny/st7586.c
>> @@ -285,7 +285,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7586_fops);
>>  static struct drm_driver st7586_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &st7586_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "st7586",
>>  	.desc			= "Sitronix ST7586",
>> diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
>> index 0af1b15efdf8a..cfd4933f3b30c 100644
>> --- a/drivers/gpu/drm/tiny/st7735r.c
>> +++ b/drivers/gpu/drm/tiny/st7735r.c
>> @@ -157,7 +157,7 @@ DEFINE_DRM_GEM_CMA_FOPS(st7735r_fops);
>>  static struct drm_driver st7735r_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>  	.fops			= &st7735r_fops,
>> -	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.debugfs_init		= mipi_dbi_debugfs_init,
>>  	.name			= "st7735r",
>>  	.desc			= "Sitronix ST7735R",
>> diff --git a/include/drm/drm_gem_cma_helper.h b/include/drm/drm_gem_cma_helper.h
>> index 947ac95eb24a9..917d42603db06 100644
>> --- a/include/drm/drm_gem_cma_helper.h
>> +++ b/include/drm/drm_gem_cma_helper.h
>> @@ -110,21 +110,37 @@ struct drm_gem_object *
>>  drm_cma_gem_create_object_default_funcs(struct drm_device *dev, size_t size);
>>  
>>  /**
>> - * DRM_GEM_CMA_VMAP_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
>> - *                               address on the buffer
>> + * __DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a
>> + *                            virtual address on the buffer
>> + * @__dumb_create: callback function for .dumb_create
>>   *
>>   * This macro provides a shortcut for setting the default GEM operations in the
>>   * &drm_driver structure for drivers that need the virtual address also on
>>   * imported buffers.
>> + *
>> + * This macro is a variant of DRM_GEM_CMA_DRIVER_OPS for drivers that
>> + * override the default implementation of .dumb_create. Use
>> + * DRM_GEM_CMA_DRIVER_OPS if possible.
>>   */
>> -#define DRM_GEM_CMA_VMAP_DRIVER_OPS \
>> +#define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
>>  	.gem_create_object	= drm_cma_gem_create_object_default_funcs, \
>> -	.dumb_create		= drm_gem_cma_dumb_create, \
>> +	.dumb_create		= (__dumb_create), \
>>  	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd, \
>>  	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle, \
>>  	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table_vmap, \
>>  	.gem_prime_mmap		= drm_gem_prime_mmap
>>  
>> +/**
>> + * DRM_GEM_CMA_DRIVER_OPS - CMA GEM driver operations ensuring a virtual
>> + *                          address on the buffer
>> + *
>> + * This macro provides a shortcut for setting the default GEM operations in the
>> + * &drm_driver structure for drivers that need the virtual address also on
>> + * imported buffers.
>> + */
>> +#define DRM_GEM_CMA_DRIVER_OPS \
>> +	__DRM_GEM_CMA_DRIVER_OPS(drm_gem_cma_dumb_create)
>> +
>>  struct drm_gem_object *
>>  drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *drm,
>>  				       struct dma_buf_attachment *attach,
>> -- 
>> 2.26.2
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH 05/21] drm/atmel-hlcdc: Use GEM CMA object functions
From: Thomas Zimmermann @ 2020-05-25 12:10 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: alexandre.belloni, linux-aspeed, narmstrong, airlied, liviu.dudau,
	dri-devel, paul, laurent.pinchart, mihail.atanassov, marex,
	khilman, abrodkin, kong.kongxinwei, xinliang.liu,
	ludovic.desroches, tomi.valkeinen, james.qian.wang, linux-imx,
	joel, alexandre.torgue, puck.chen, s.hauer, alison.wang, jsarha,
	wens, vincent.abriou, linux-arm-kernel, mcoquelin.stm32,
	bbrezillon, andrew, philippe.cornu, yannick.fertre,
	kieran.bingham+renesas, kernel, zourongrong, shawnguo
In-Reply-To: <20200522180824.GB1087580@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 3182 bytes --]

Hi

Am 22.05.20 um 20:08 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Fri, May 22, 2020 at 03:52:30PM +0200, Thomas Zimmermann wrote:
>> The atmel-hlcdc driver uses the default implementation for CMA functions. The
>> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
>> All remaining operations are provided by CMA GEM object functions.
> 
> A nice side-effect of introducing the defualt implementation
> of CMA functions is that this driver is now migrated over to use
> drm_gem_object_funcs, which is why we can replace all these
> helpers with a simple macro that defined only 6 helpers.
> 
> With an improved changelog:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> And as I said in the other mail, this is really nice.
> It is now much more obvious that this drivers uses
> all the default helpers for CMA.
> 
> And I can drop one item from my TODO list on top of that.

There's still more to do. The current macro still sets
.gem_object_create to a CMA default. But that pointer is the interface
where drivers can override some of the CMA object defaults, so it should
not be set by CMA helpers.

After the other CMA drivers have been converted to GEM object functions,
this setting can be kept to zero and __drm_gem_cma_create() can set the
default object functions. SHMEM and VRAM helpers already work this way.

Best regards
Thomas

> 
> 	Sam
> 
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 11 +----------
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index 112aa5066ceed..871293d1aeeba 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -821,16 +821,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>>  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>>  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>>  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
>> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
>> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
>> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
>> -	.dumb_create = drm_gem_cma_dumb_create,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.fops = &fops,
>>  	.name = "atmel-hlcdc",
>>  	.desc = "Atmel HLCD Controller DRM",
>> -- 
>> 2.26.2
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH v5 4/8] dt-bindings: display: panel: Add ilitek ili9341 panel bindings
From: Linus Walleij @ 2020-05-25 12:20 UTC (permalink / raw)
  To: dillon min
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, linux-kernel@vger.kernel.org,
	open list:DRM PANEL DRIVERS, linux-spi, Mark Brown, linux-stm32,
	Linux ARM
In-Reply-To: <1590378348-8115-5-git-send-email-dillon.minfei@gmail.com>

On Mon, May 25, 2020 at 5:46 AM <dillon.minfei@gmail.com> wrote:

> From: dillon min <dillon.minfei@gmail.com>
>
> Add documentation for "ilitek,ili9341" panel.
>
> Signed-off-by: dillon min <dillon.minfei@gmail.com>

This looks good to me!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

^ permalink raw reply

* Re: [PATCH v5 6/8] drm/panel: Add ilitek ili9341 panel driver
From: Linus Walleij @ 2020-05-25 12:23 UTC (permalink / raw)
  To: dillon min
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-clk, linux-kernel@vger.kernel.org,
	open list:DRM PANEL DRIVERS, linux-spi, Mark Brown, linux-stm32,
	Linux ARM
In-Reply-To: <1590378348-8115-7-git-send-email-dillon.minfei@gmail.com>

On Mon, May 25, 2020 at 5:46 AM <dillon.minfei@gmail.com> wrote:

> From: dillon min <dillon.minfei@gmail.com>
>
>     This driver combine tiny/ili9341.c mipi_dbi_interface driver
>     with mipi_dpi_interface driver, can support ili9341 with serial
>     mode or parallel rgb interface mode by register configuration.
>
> Changes since V3:
>
>     accoding to Linus Walleij's suggestion.
>     1 add more comments to driver.
>     2 reduce magic number usage in the driver.
>     3 move panel configuration from common place to system configuration.
>     4 reuse MIPI_DCS_* as more as possible.
>
> Signed-off-by: dillon min <dillon.minfei@gmail.com>

This looks good to me!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

^ permalink raw reply

* Re: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2020-05-25 12:24 UTC (permalink / raw)
  To: Jerry-ch Chen
  Cc: linux-devicetree, Sean Cheng (鄭昇弘),
	Laurent Pinchart, Rynn Wu (吳育恩), zwisler,
	srv_heupstream, Christie Yu (游雅惠),
	Hans Verkuil, Jungo Lin (林明俊), Sj Huang,
	yuzhao, Hans Verkuil, Pi-Hsun Shih,
	Frederic Chen (陳俊元), Matthias Brugger,
	moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
	list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
	Linux Media Mailing List
In-Reply-To: <1590156658.27807.84.camel@mtksdccf07>

r

On Fri, May 22, 2020 at 4:11 PM Jerry-ch Chen
<Jerry-ch.Chen@mediatek.com> wrote:
>
> Hi Tomasz,
>
> On Thu, 2020-05-21 at 18:28 +0000, Tomasz Figa wrote:
> > Hi Jerry,
> >
> > On Wed, Dec 04, 2019 at 08:47:32PM +0800, Jerry-ch Chen wrote:
[snip]
> > > +
> > > +enum face_angle {
> > > +   MTK_FD_FACE_FRONT,
> > > +   MTK_FD_FACE_RIGHT_50,
> > > +   MTK_FD_FACE_LEFT_50,
> > > +   MTK_FD_FACE_RIGHT_90,
> > > +   MTK_FD_FACE_LEFT_90,
> > > +   MTK_FD_FACE_ANGLE_NUM,
> > > +};
> >
> > This enum seems to define values for the V4L2_CID_MTK_FD_DETECT_POSE
> > control. Considering that this is an enumeration and the values are
> > actually integers (-90, -50, 0, 50, 90), perhaps this should be an
> > INTEGER_MENU control instead?
> >
>
> this ioctl let user select multiple face positions(combination of angles
> and directions) to be detected. so I thought I am not able to use the
> INTEGER_MENU for this purpose.

Ah, okay, I thought there is only one selection possible.

>
> A bit-field as following should be used by user.
> I consider adding it to uapi.
>
> struct face_direction_def {
> __u16 MTK_FD_FACE_DIR_0 : 1,
>         MTK_FD_FACE_DIR_30 : 1,
>         MTK_FD_FACE_DIR_60 : 1,
>         MTK_FD_FACE_DIR_90 : 1,
>         MTK_FD_FACE_DIR_120 : 1,
>         MTK_FD_FACE_DIR_150 : 1,
>         MTK_FD_FACE_DIR_180 : 1,
>         MTK_FD_FACE_DIR_210 : 1,
>         MTK_FD_FACE_DIR_240 : 1,
>         MTK_FD_FACE_DIR_270 : 1,
>         MTK_FD_FACE_DIR_300 : 1,
>         MTK_FD_FACE_DIR_330 : 1,
>         : 4;
> };

Note that bit fields are not recommended in UAPI because of not being
well specified by the language. Instead bits should be defined using
macros with explicit masks or sometimes enums.

>
> User can also select some face directions of each face angle in one
> ioctl, for example:
>
> /*
>  * u16 face_directions[MTK_FD_FACE_ANGLE_NUM] = {0};
>  *
>  *      face_directions[MTK_FD_FACE_FRONT] = 0x7; //angle:0, dir:0,30,60
>  *      face_directions[MTK_FACE_RIGHT_50] = 0x2; //angle:50, dir:30
>  *
>  */

Makes sense, thanks.

>
> > > +
> > > +struct fd_buffer {
> > > +   __u32 scp_addr; /* used by SCP */
> > > +   __u32 dma_addr; /* used by DMA HW */
> > > +} __packed;
> fd buffer is used for scp ipi
>
> > > +
> > > +struct fd_face_result {
> > > +   char data[16];
> > > +};
> fd_face_result is used for user, so it should be moved to
> include/uapi/linux.
> In fact, it has bit-field definition for user, so I would like to define
> it in include/uapi/linux as following:
>
> struct fd_face_result {
>   __u64 face_idx : 12,
>         type : 1,
>         x0 : 10,
>         y0 : 10,
>         x1 : 10,
>         y1 : 10,
>         fcv1 : 11;
>   __u64 fcv2 : 7,
>         rip_dir : 4,
>         rop_dir : 3,
>         det_size : 5;
> };
>

Indeed this should be defined, but as per my comment above, please
avoid using the bitfield construct and define shifts and masks
instead.

>
> > > +
> > > +struct fd_user_output {
> > > +   struct fd_face_result results[MTK_FD_MAX_RESULT_NUM];
> > > +   __u16 number;
> >
> > Is this perhaps the number of results? If so, would num_results be a better
> > name?
> >
> yes, fixed.
> > > +};
> >
> > Since this struct is the meta buffer format, it is a part of the userspace
> > interface and should be defined in a header under include/uapi/linux/.
> >
> Ok, I will create include/uapi/linux/mtk_fd_40.h
> which suppose to include structures that userspace will use.
> should the private IOCTLs be placed in it together?
>

Sorry, what private IOCTLs are you referring to? If you mean private
control IDs, then yes, they should go to that header.

[snip]
> > > +static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> > > +                             unsigned int *num_buffers,
> > > +                             unsigned int *num_planes,
> > > +                             unsigned int sizes[],
> > > +                             struct device *alloc_devs[])
> > > +{
> > > +   struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > +   unsigned int size[2];
> > > +   unsigned int plane;
> > > +
> > > +   switch (vq->type) {
> > > +   case V4L2_BUF_TYPE_META_CAPTURE:
> > > +           size[0] = ctx->dst_fmt.buffersize;
> > > +           break;
> > > +   case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > > +           size[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
> > > +           if (*num_planes == 2)
> > > +                   size[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
> > > +           break;
> > > +   }
> >
> > Is this code above needed? The code below sets sizes[] and it uses a for loop,
> > without opencoded assignment for the second plane.
> >
>
> Looks like not really useful here,
> it should check sizes and num_planes if num_plane not zero,
> and for V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, it will at most have 2
> planes, maybe no need for loop as well.

Loops generally make the code cleaner and there might be some desire
to add support for more formats in the future, e.g. in case a next
generation of the hardware shows up.

> I will refine this function as following:
> mtk_fd_vb2_queue_setup(...)
> {
>         struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
>
>         if (*num_planes == 0) {
>                 if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
>                         sizes[0] = ctx->dst_fmt.buffersize;
>                         *num_planes = 1;
>                         return 0;
>                 } else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
>                         *num_planes = ctx->src_fmt.num_planes;
>                         sizes[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
>                         if (*num_planes == 2)
>                                 sizes[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
>                         return 0;
>                 }
>                 return -EINVAL;
>         }
>
>         /* If num_plane not zero, check the num_plane and sizes*/
>         if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
>                 if ((*num_planes == 1) &&
>                     (sizes[0] <= ctx->dst_fmt.buffersize))
>                         return 0;

nit: The typical convention is to check for problems and return the
error code earlier, with the success handled at the end of the block.

>                 else
>                         return -EINVAL;
>         }
>         if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
>                 if ((*num_planes == 1) &&
>                     (sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage))
>                         return 0;
>                 else if ((*num_planes == 2) &&
>                          (sizes[0] <= ctx->src_fmt.plane_fmt[0].sizeimage) &&
>                          (sizes[1] <= ctx->src_fmt.plane_fmt[1].sizeimage))
>                         return 0;

Wouldn't a loop eliminate the need to if/else if through the various
supported cases and duplicate the size checks?

>                 else
>                         return -EINVAL;
>
>         }
>         return 0;
> }

How about the following?

mtk_fd_vb2_queue_setup(...)
{
        struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);

        if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
                if (*num_planes == 0) {
                        *num_planes = 1;
                        sizes[0] = ctx->dst_fmt.buffersize;
                        return 0;
                }

                if (*num_planes != 1 || sizes[0] < ctx->dst_fmt.buffersize)
                                return -EINVAL;

                return 0;
        }

        /* V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE */
        if (*num_planes == 0) {
                        *num_planes = ctx->src_fmt.num_planes;
                        for (i = 0; i < ctx->src_fmt.num_planes; ++i)
                                sizes[i] = ctx->src_fmt.plane_fmt[i].sizeimage;
                        return 0;
        }

        if (*num_planes < ctx->src_fmt.num_planes)
                return -EINVAL;

        for (i = 0; i < ctx->src_fmt.num_planes; ++i)
                if (sizes[i] < ctx->src_fmt.plane_fmt[i].sizeimage)
                        return -EINVAL;

        return 0;
}

Note that it fully separates the code dealing with each queue and thus
improves the readability.

In this case, it could actually be beneficial to split the vb2_ops
implementation into one that deals only with video_output_mplane and
one only with meta_capture. This would allow eliminating the special
casing based on vq->type and thus further simplify the code. Not sure
if it applies to the other vb2 callbacks, though.

[snip]
> > > +static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> > > +                             const struct v4l2_pix_format_mplane *sfmt)
> > > +{
> > > +   dfmt->field = V4L2_FIELD_NONE;
> > > +   dfmt->colorspace = V4L2_COLORSPACE_BT2020;
> > > +   dfmt->num_planes = sfmt->num_planes;
> > > +   dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > > +   dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > > +   dfmt->xfer_func =
> > > +           V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> > > +
> > > +   /* Keep user setting as possible */
> > > +   dfmt->width = clamp(dfmt->width,
> > > +                       MTK_FD_OUTPUT_MIN_WIDTH,
> > > +                       MTK_FD_OUTPUT_MAX_WIDTH);
> > > +   dfmt->height = clamp(dfmt->height,
> > > +                        MTK_FD_OUTPUT_MIN_HEIGHT,
> > > +                        MTK_FD_OUTPUT_MAX_HEIGHT);
> > > +
> > > +   if (sfmt->num_planes == 2) {
> > > +           /* NV16M and NV61M has 1 byte per pixel */
> > > +           dfmt->plane_fmt[0].bytesperline = dfmt->width;
> > > +           dfmt->plane_fmt[1].bytesperline = dfmt->width;
> > > +   } else {
> > > +           /* 2 bytes per pixel */
> > > +           dfmt->plane_fmt[0].bytesperline = dfmt->width * 2;
> > > +   }
> > > +
> > > +   dfmt->plane_fmt[0].sizeimage =
> > > +           dfmt->height * dfmt->plane_fmt[0].bytesperline;
> >
> > Could some of the code above be replaced with v4l2_fill_pixfmt_mp()?
> >
> I would like to refine as following
>
> mtk_fd_fill_pixfmt_mp(...){
>         v4l2_fill_pixfmt_mp(dfmt, sfmt->pixelformat, dfmt->width,
> dfmt->height);
>
>         dfmt->field = V4L2_FIELD_NONE;
>         dfmt->colorspace = V4L2_COLORSPACE_BT2020;
>         dfmt->num_planes = sfmt->num_planes;

num_planes should be already filled in by the helper. That actually
raises a question on whether there is any need to have sfmt passed to
this function at all. Perhaps all we need is the value of pixelformat?

>         dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>         dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>         dfmt->xfer_func =
>                 V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> }
>

Isn't still a need to clamp() width and height to min/max, though?

[snip]
> > > +   fd_param.user_param.src_img_fmt =
> > > +           get_fd_img_fmt(ctx->src_fmt.pixelformat);
> > > +   if (ctx->src_fmt.num_planes == 2)
> > > +           fd_param.src_img[1].dma_addr =
> > > +                   vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);
> >
> > nit: Could this be moved above, to be just below src_img[0] initialization,
> > for readability reasons?
> >
> Ok, this function will be refined as
>
> static void mtk_fd_device_run(void *priv)
> {
>         struct mtk_fd_ctx *ctx = priv;
>         struct mtk_fd_dev *fd = ctx->fd_dev;
>         struct vb2_v4l2_buffer *src_buf, *dst_buf;
>         struct fd_enq_param fd_param;
>
>         src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>         dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
>         fd_param.src_img[0].dma_addr =
>                 vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
>         if (ctx->src_fmt.num_planes == 2)
>                 fd_param.src_img[1].dma_addr =
>                         vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 1);

How about making this a loop in terms of ctx->src_fmt.num_planes?

>         fd_param.user_result.dma_addr =
>                 vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
>         fd_param.user_param.src_img_fmt =
>                 get_fd_img_fmt(fd->dev, ctx->src_fmt.pixelformat);
>
>         mtk_fd_fill_user_param(&fd_param.user_param, &ctx->hdl);
>
>         /* Complete request controls if any */
>         v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req, &ctx->hdl);
>
>         fd->output = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
>         mtk_fd_hw_job_exec(fd, &fd_param);
> }

Best regards,
Tomasz

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

^ permalink raw reply

* Re: [PATCH 05/21] drm/atmel-hlcdc: Use GEM CMA object functions
From: Thomas Zimmermann @ 2020-05-25 12:37 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: alexandre.belloni, linux-aspeed, narmstrong, airlied,
	linus.walleij, liviu.dudau, stefan, philippe.cornu, paul,
	laurent.pinchart, benjamin.gaignard, mihail.atanassov, festevam,
	alexandre.torgue, marex, khilman, abrodkin, ludovic.desroches,
	xinliang.liu, kong.kongxinwei, tomi.valkeinen, james.qian.wang,
	joel, linux-imx, p.zabel, puck.chen, s.hauer, alison.wang,
	maarten.lankhorst, mripard, john.stultz, jsarha, wens,
	vincent.abriou, kernel, linux-arm-kernel, mcoquelin.stm32, noralf,
	bbrezillon, andrew, dri-devel, yannick.fertre,
	kieran.bingham+renesas, daniel, zourongrong, shawnguo,
	brian.starkey
In-Reply-To: <20200522192546.GA1516695@ravnborg.org>


[-- Attachment #1.1.1: Type: text/plain, Size: 2513 bytes --]

Hi

Am 22.05.20 um 21:25 schrieb Sam Ravnborg:
> Hi Thomas.
> 
> On Fri, May 22, 2020 at 03:52:30PM +0200, Thomas Zimmermann wrote:
>> The atmel-hlcdc driver uses the default implementation for CMA functions. The
>> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
>> All remaining operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 11 +----------
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> index 112aa5066ceed..871293d1aeeba 100644
>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
>> @@ -821,16 +821,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>>  	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
>>  	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
>>  	.irq_uninstall = atmel_hlcdc_dc_irq_uninstall,
>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>> -	.gem_vm_ops = &drm_gem_cma_vm_ops,
>> -	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>> -	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>> -	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>> -	.gem_prime_vmap = drm_gem_cma_prime_vmap,
>> -	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
> 
>> -	.gem_prime_mmap = drm_gem_cma_prime_mmap,
> When using DRM_GEM_CMA_DRIVER_OPS gem_prime_mmap is set to
> drm_gem_prime_mmap.
> Why is this the same as drm_gem_cma_prime_mmap?
> 
> Maybe this is all obvious when you know all the CMA stuff,
> but this puzzeled me.

Following through the calls is far from easy.

I took the macro from the aspeed driver. I had some doubts about the
mmap code, but expected the driver to be working correctly. Maybe we
should set that field to drm_gem_cma_prime_mmap or implement the mmap
object function.

Best regards
Thomas



> 
> 	Sam
> 
> 
>> -	.dumb_create = drm_gem_cma_dumb_create,
>> +	DRM_GEM_CMA_DRIVER_OPS,
>>  	.fops = &fops,
>>  	.name = "atmel-hlcdc",
>>  	.desc = "Atmel HLCD Controller DRM",
>> -- 
>> 2.26.2

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH v5 2/3] dt-bindings: arm: actions: Document Caninos Loucos Labrador
From: Matheus Castello @ 2020-05-25 12:38 UTC (permalink / raw)
  To: Andreas Färber, manivannan.sadhasivam, robh+dt
  Cc: mark.rutland, devicetree, Rob Herring, linux-actions,
	linux-kernel, igor.lima, edgar.righi, linux-arm-kernel
In-Reply-To: <8fc7b0d0-6516-ecd4-ce9c-a63a3cba7e9a@suse.de>

Hi Andreas,

Em 5/25/20 7:41 AM, Andreas Färber escreveu:
> Hi,
> 
> Am 25.05.20 um 03:30 schrieb Matheus Castello:
>> Update the documentation to add the Caninos Loucos Labrador. Labrador
>> project consists of a computer on module based on the Actions Semi S500
>> processor and the Labrador base board.
>>
>> Signed-off-by: Matheus Castello <matheus@castello.eng.br>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>   Documentation/devicetree/bindings/arm/actions.yaml | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/actions.yaml 
>> b/Documentation/devicetree/bindings/arm/actions.yaml
>> index ace3fdaa8396..2187e1c5bc73 100644
>> --- a/Documentation/devicetree/bindings/arm/actions.yaml
>> +++ b/Documentation/devicetree/bindings/arm/actions.yaml
>> @@ -19,6 +19,11 @@ properties:
>>                 - allo,sparky # Allo.com Sparky
>>                 - cubietech,cubieboard6 # Cubietech CubieBoard6
>>             - const: actions,s500
>> +      - items:
>> +          - enum:
>> +              - caninos,labrador-v2 # Labrador Core v2
>> +              - caninos,labrador-base-m # Labrador Base Board M v1
> 
> This enum still strikes me as wrong, it means either-or. (Was planning 
> to look into it myself, but no time yet...) caninos,labrador-v2 should 
> be a const one level down: board, SoM, SoC from most specific to most 
> generic. Compare Guitar below.

I got it, I agree make sense, I will send the v6.

> 
>> +          - const: actions,s500
>>         - items:
>>             - enum:
>>                 - lemaker,guitar-bb-rev-b # LeMaker Guitar Base Board 
>> rev. B
> 
> Regards,
> Andreas
> 

Best Regards,
Matheus Castello

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

^ permalink raw reply

* Re: [PATCH V4 00/17] arm64/cpufeature: Introduce ID_PFR2, ID_DFR1, ID_MMFR5 and other changes
From: Anshuman Khandual @ 2020-05-25 12:39 UTC (permalink / raw)
  To: Will Deacon, linux-arm-kernel; +Cc: catalin.marinas, kvmarm, linux-kernel, maz
In-Reply-To: <159007243559.201193.1705879548179051424.b4-ty@kernel.org>



On 05/21/2020 08:49 PM, Will Deacon wrote:
> On Tue, 19 May 2020 15:10:37 +0530, Anshuman Khandual wrote:
>> This series is primarily motivated from an adhoc list from Mark Rutland
>> during our previous ID_ISAR6 discussion [1]. The current proposal also
>> accommodates some more suggestions from Will and Suzuki.
>>
>> This series adds missing 32 bit system registers (ID_PFR2, ID_DFR1 and
>> ID_MMFR5), adds missing features bits on all existing system registers
>> (32 and 64 bit) and some other miscellaneous changes. While here it also
>> includes a patch which does macro replacement for various open bits shift
>> encodings for various CPU ID registers. There is a slight re-order of the
>> patches here as compared to the previous version (V1).
>>
>> [...]
> 
> Applied to arm64 (for-next/cpufeature), thanks!
> 
> [01/17] arm64/cpufeature: Add explicit ftr_id_isar0[] for ID_ISAR0 register
>         https://git.kernel.org/arm64/c/2a5bc6c47bc3
> [02/17] arm64/cpufeature: Drop TraceFilt feature exposure from ID_DFR0 register
>         https://git.kernel.org/arm64/c/1ed1b90a0594
> [03/17] arm64/cpufeature: Make doublelock a signed feature in ID_AA64DFR0
>         https://git.kernel.org/arm64/c/e965bcb06256
> [04/17] arm64/cpufeature: Introduce ID_PFR2 CPU register
>         https://git.kernel.org/arm64/c/16824085a7dd
> [05/17] arm64/cpufeature: Introduce ID_DFR1 CPU register
>         https://git.kernel.org/arm64/c/dd35ec070457
> [06/17] arm64/cpufeature: Introduce ID_MMFR5 CPU register
>         https://git.kernel.org/arm64/c/152accf8476f
> [07/17] arm64/cpufeature: Add remaining feature bits in ID_PFR0 register
>         https://git.kernel.org/arm64/c/0ae43a99fe91
> [08/17] arm64/cpufeature: Add remaining feature bits in ID_MMFR4 register
>         https://git.kernel.org/arm64/c/fcd6535322cc
> [09/17] arm64/cpufeature: Add remaining feature bits in ID_AA64ISAR0 register
>         https://git.kernel.org/arm64/c/7cd51a5a84d1
> [10/17] arm64/cpufeature: Add remaining feature bits in ID_AA64PFR0 register
>         https://git.kernel.org/arm64/c/011e5f5bf529
> [11/17] arm64/cpufeature: Add remaining feature bits in ID_AA64PFR1 register
>         https://git.kernel.org/arm64/c/14e270fa5c4c
> [12/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR0 register
>         (no commit info)
> [13/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR1 register
>         (no commit info)
> [14/17] arm64/cpufeature: Add remaining feature bits in ID_AA64MMFR2 register
>         (no commit info)
> [15/17] arm64/cpufeature: Add remaining feature bits in ID_AA64DFR0 register
>         (no commit info)
> [16/17] arm64/cpufeature: Replace all open bits shift encodings with macros
>         (no commit info)
> [17/17] arm64/cpuinfo: Add ID_MMFR4_EL1 into the cpuinfo_arm64 context
>         https://git.kernel.org/arm64/c/858b8a8039d0
> 
> Note that Suzuki had comments on 12-16, so assume you'll respin those (I fixed
> up the trivial comments on earlier patches myself).
[PATCH 15/17] might need some more investigation and rework. Hence planning to
defer that for later and respin the remaining patches (12, 13, 14, 16) for now.

- Anshuman

_______________________________________________
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 07/21] drm/hisilicon/kirin: Use GEM CMA object functions
From: Thomas Zimmermann @ 2020-05-25 12:41 UTC (permalink / raw)
  To: Emil Velikov
  Cc: alexandre.belloni, linux-aspeed, Neil Armstrong, Dave Airlie,
	Liviu Dudau, ML dri-devel, Paul Cercueil, Laurent Pinchart,
	Mihail Atanassov, Sam Ravnborg, Marek Vašut, khilman,
	Alexey Brodkin, Xinwei Kong, Xinliang Liu, ludovic.desroches,
	Tomi Valkeinen, james qian wang (Arm Technology China),
	NXP Linux Team, joel, Alexandre Torgue, Chen Feng, Sascha Hauer,
	Alison Wang, Jyri Sarha, Chen-Yu Tsai, Vincent Abriou, LAKML,
	Maxime Coquelin, bbrezillon, andrew, Philippe Cornu,
	Yannick Fertre, Kieran Bingham, Sascha Hauer, Rongrong Zou,
	Shawn Guo
In-Reply-To: <CACvgo51cYh4iLKEfrLSbgOGQM4=ojsBq54gW9VJBPoX+p04o+g@mail.gmail.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 2335 bytes --]

Hi Emil

Am 22.05.20 um 20:11 schrieb Emil Velikov:
> Hi Thomas,
> 
> On Fri, 22 May 2020 at 14:53, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> The kirin driver uses the default implementation for CMA functions; except
>> for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro now sets
>> these defaults and .dumb_create in struct drm_driver. All remaining
>> operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 12 +-----------
>>  1 file changed, 1 insertion(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
>> index c339e632522a9..b1ffd7d43e562 100644
>> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
>> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
>> @@ -921,17 +921,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ade_fops);
>>  static struct drm_driver ade_driver = {
>>         .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>         .fops = &ade_fops,
>> -       .gem_free_object_unlocked = drm_gem_cma_free_object,
>> -       .gem_vm_ops = &drm_gem_cma_vm_ops,
>> -       .dumb_create = drm_gem_cma_dumb_create_internal,
> 
> This doesn't seem right. The _internal documentation explicitly says
> that this should _not_ be used as .dumb_create. Instead drivers should
> use it to implement their callback.
> 
> Since it yields the same result as drm_gem_cma_dumb_create we can use
> the default macro below.

I noticed this and thought that the driver authors probably had their
reasons. Changing the driver to the default macro is probably still a
good idea.

Best regards
Thomas

> 
> Weather to the .dumb_create in separate patch, or squash it here -
> I'll leave to you.
> In case of the latter, please mentioned it in the commit message.
> 
> -Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions
From: Thomas Zimmermann @ 2020-05-25 12:49 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: alexandre.belloni, linux-aspeed, narmstrong, airlied, liviu.dudau,
	dri-devel, paul, mihail.atanassov, sam, marex, khilman, abrodkin,
	kong.kongxinwei, xinliang.liu, ludovic.desroches, tomi.valkeinen,
	james.qian.wang, joel, linux-imx, alexandre.torgue, puck.chen,
	s.hauer, alison.wang, jsarha, wens, vincent.abriou,
	linux-arm-kernel, mcoquelin.stm32, bbrezillon, andrew,
	philippe.cornu, yannick.fertre, kieran.bingham+renesas, kernel,
	zourongrong, shawnguo
In-Reply-To: <20200522201240.GE5824@pendragon.ideasonboard.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 3887 bytes --]

Hi

Am 22.05.20 um 22:12 schrieb Laurent Pinchart:
> Hi Thomas,
> 
> Thank you for the patch.
> 
> On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote:
>> The rcar-du driver uses the default implementation for CMA functions;
>> except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro
>> now sets these defaults and .dumb_create in struct drm_driver. All
>> remaining operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +----------
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> index 3e67cf70f0402..3728038cec1d1 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
>>  
>>  static struct drm_driver rcar_du_driver = {
>>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>> -	.gem_free_object_unlocked = drm_gem_cma_free_object,
>> -	.gem_vm_ops		= &drm_gem_cma_vm_ops,
>> -	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
>> -	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
>> -	.gem_prime_get_sg_table	= drm_gem_cma_prime_get_sg_table,
>> -	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
>> -	.gem_prime_vmap		= drm_gem_cma_prime_vmap,
>> -	.gem_prime_vunmap	= drm_gem_cma_prime_vunmap,
>> -	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
>> -	.dumb_create		= rcar_du_dumb_create,
>> +	__DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create),
> 
> Your __DRM_GEM_CMA_DRIVER_OPS is defined as
> 
> #define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \
>         .gem_create_object      = drm_cma_gem_create_object_default_funcs, \
>         .dumb_create            = (__dumb_create), \
>         .prime_handle_to_fd     = drm_gem_prime_handle_to_fd, \
>         .prime_fd_to_handle     = drm_gem_prime_fd_to_handle, \
>         .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table_vmap, \
>         .gem_prime_mmap         = drm_gem_prime_mmap
> 
> The patch thus introduces several changes:
> 
> - drm_gem_cma_prime_import_sg_table_vmap() is used instead of
>   drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap()
>   and .gem_prime_vunmap(). I believe that's fine, but splitting that
>   change in a separate commit, or at the very least explaining it in
>   details in the commit message, would make review easier.
> 
> - .gem_create_object() is now set. That seems to be OK, but I'm not sure
>   to grasp all the implications. This should also be explained in the
>   commit message, and ideally split to a separate patch.

That's relevant during object creation and sets the object functions.
See one of my other replies for how this can go away after all CMA
drivers have been updated to GEM object functions.


> 
> - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same
>   comments :-)

I relied on the aspeed driver to be correct. After Sam's comment on
that, I read the code once again several times. The original
implementation clears VM_PFNMAP. And I cannot find that code any longer.
Going back to the original function might be better.


> 
> This patch hides way too many changes in what is documented as just
> innocent refactoring. It seems other drivers are affected too.

Could you test the patchset? I don't have the HW.

Best regards
Thomas

> 
>>  	.fops			= &rcar_du_fops,
>>  	.name			= "rcar-du",
>>  	.desc			= "Renesas R-Car Display Unit",
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

^ permalink raw reply

* Re: [PATCH 12/21] drm/mcde: Use GEM CMA object functions
From: Thomas Zimmermann @ 2020-05-25 12:51 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Belloni, linux-aspeed, Neil Armstrong, Dave Airlie,
	Liviu Dudau, Stefan Agner, Philippe Cornu, Paul Cercueil,
	Laurent Pinchart, Benjamin Gaignard, Mihail Atanassov,
	Sam Ravnborg, Alexandre TORGUE, Marek Vasut, Fabio Estevam,
	abrodkin, Ludovic Desroches, Xinliang Liu, k00278426,
	Tomi Valkeinen, james.qian.wang, Joel Stanley, NXP Linux Team,
	Philipp Zabel, Chenfeng (puck), Sascha Hauer, Alison Wang,
	Maarten Lankhorst, Maxime Ripard, John Stultz, Jyri Sarha,
	Chen-Yu Tsai, Vincent Abriou, Sascha Hauer, Linux ARM,
	Maxime Coquelin, Noralf Trønnes, Boris Brezillon,
	Andrew Jeffery, open list:DRM PANEL DRIVERS, Yannick Fertre,
	Kieran Bingham, Daniel Vetter, Kevin Hilman, Rongrong Zou,
	Shawn Guo, Brian Starkey
In-Reply-To: <CACRpkdaHuRHiNjisai2d-karW6y11M2qsPkx5Hn0sfc8T6B_qQ@mail.gmail.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 797 bytes --]

Hi

Am 25.05.20 um 13:36 schrieb Linus Walleij:
> On Fri, May 22, 2020 at 3:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 
>> The mcde driver uses the default implementation for CMA functions. The
>> DRM_GEM_CMA_DRIVER_OPS macro now sets these defaults in struct drm_driver.
>> All remaining operations are provided by CMA GEM object functions.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Could you boot-test with the patchset applied?

Best regards
Thomas

> 
> Yours,
> Linus Walleij
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

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

^ permalink raw reply

* [PATCH v3 2/6] arm64: Add level-hinted TLB invalidation helper
From: Zhenyu Ye @ 2020-05-25 12:52 UTC (permalink / raw)
  To: catalin.marinas, peterz, mark.rutland, will, aneesh.kumar, akpm,
	npiggin, arnd, rostedt, maz, suzuki.poulose, tglx, yuzhao,
	Dave.Martin, steven.price, broonie, guohanjun
  Cc: linux-arch, yezhenyu2, linux-kernel, xiexiangyou, zhangshaokun,
	linux-mm, arm, prime.zeng, kuhn.chenqun, linux-arm-kernel
In-Reply-To: <20200525125300.794-1-yezhenyu2@huawei.com>

From: Marc Zyngier <maz@kernel.org>

Add a level-hinted TLB invalidation helper that only gets used if
ARMv8.4-TTL gets detected.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/tlbflush.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
index bc3949064725..8adbd6fd8489 100644
--- a/arch/arm64/include/asm/tlbflush.h
+++ b/arch/arm64/include/asm/tlbflush.h
@@ -10,6 +10,7 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/bitfield.h>
 #include <linux/mm_types.h>
 #include <linux/sched.h>
 #include <asm/cputype.h>
@@ -59,6 +60,34 @@
 		__ta;						\
 	})
 
+#define TLBI_TTL_MASK	GENMASK_ULL(47, 44)
+
+#define __tlbi_level(op, addr, level) do {			\
+	u64 arg = addr;						\
+								\
+	if (cpus_have_const_cap(ARM64_HAS_ARMv8_4_TTL) &&	\
+	    level) {						\
+		u64 ttl = level;				\
+								\
+		switch (PAGE_SIZE) {				\
+		case SZ_4K:					\
+			ttl |= 1 << 2;				\
+			break;					\
+		case SZ_16K:					\
+			ttl |= 2 << 2;				\
+			break;					\
+		case SZ_64K:					\
+			ttl |= 3 << 2;				\
+			break;					\
+		}						\
+								\
+		arg &= ~TLBI_TTL_MASK;				\
+		arg |= FIELD_PREP(TLBI_TTL_MASK, ttl);		\
+	}							\
+								\
+	__tlbi(op,  arg);					\
+} while (0)
+
 /*
  *	TLB Invalidation
  *	================
-- 
2.19.1



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

^ permalink raw reply related

* [PATCH v3 1/6] arm64: Detect the ARMv8.4 TTL feature
From: Zhenyu Ye @ 2020-05-25 12:52 UTC (permalink / raw)
  To: catalin.marinas, peterz, mark.rutland, will, aneesh.kumar, akpm,
	npiggin, arnd, rostedt, maz, suzuki.poulose, tglx, yuzhao,
	Dave.Martin, steven.price, broonie, guohanjun
  Cc: linux-arch, yezhenyu2, linux-kernel, xiexiangyou, zhangshaokun,
	linux-mm, arm, prime.zeng, kuhn.chenqun, linux-arm-kernel
In-Reply-To: <20200525125300.794-1-yezhenyu2@huawei.com>

From: Marc Zyngier <maz@kernel.org>

In order to reduce the cost of TLB invalidation, the ARMv8.4 TTL
feature allows TLBs to be issued with a level allowing for quicker
invalidation.

Let's detect the feature for now. Further patches will implement
its actual usage.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/cpucaps.h |  3 ++-
 arch/arm64/include/asm/sysreg.h  |  1 +
 arch/arm64/kernel/cpufeature.c   | 11 +++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 8eb5a088ae65..cabb0c49a1d1 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -61,7 +61,8 @@
 #define ARM64_HAS_AMU_EXTN			51
 #define ARM64_HAS_ADDRESS_AUTH			52
 #define ARM64_HAS_GENERIC_AUTH			53
+#define ARM64_HAS_ARMv8_4_TTL			54
 
-#define ARM64_NCAPS				54
+#define ARM64_NCAPS				55
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index c4ac0ac25a00..477d84ba1056 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -725,6 +725,7 @@
 
 /* id_aa64mmfr2 */
 #define ID_AA64MMFR2_E0PD_SHIFT		60
+#define ID_AA64MMFR2_TTL_SHIFT		48
 #define ID_AA64MMFR2_FWB_SHIFT		40
 #define ID_AA64MMFR2_AT_SHIFT		32
 #define ID_AA64MMFR2_LVA_SHIFT		16
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9fac745aa7bb..d993dc6dc7d5 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -244,6 +244,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
 
 static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_E0PD_SHIFT, 4, 0),
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_TTL_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_FWB_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_AT_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_LVA_SHIFT, 4, 0),
@@ -1622,6 +1623,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_cpuid_feature,
 		.cpu_enable = cpu_has_fwb,
 	},
+	{
+		.desc = "ARMv8.4 Translation Table Level",
+		.type = ARM64_CPUCAP_SYSTEM_FEATURE,
+		.capability = ARM64_HAS_ARMv8_4_TTL,
+		.sys_reg = SYS_ID_AA64MMFR2_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR2_TTL_SHIFT,
+		.min_field_value = 1,
+		.matches = has_cpuid_feature,
+	},
 #ifdef CONFIG_ARM64_HW_AFDBM
 	{
 		/*
-- 
2.19.1



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

^ permalink raw reply related

* [PATCH v3 0/6] arm64: tlb: add support for TTL feature
From: Zhenyu Ye @ 2020-05-25 12:52 UTC (permalink / raw)
  To: catalin.marinas, peterz, mark.rutland, will, aneesh.kumar, akpm,
	npiggin, arnd, rostedt, maz, suzuki.poulose, tglx, yuzhao,
	Dave.Martin, steven.price, broonie, guohanjun
  Cc: linux-arch, yezhenyu2, linux-kernel, xiexiangyou, zhangshaokun,
	linux-mm, arm, prime.zeng, kuhn.chenqun, linux-arm-kernel

In order to reduce the cost of TLB invalidation, ARMv8.4 provides
the TTL field in TLBI instruction.  The TTL field indicates the
level of page table walk holding the leaf entry for the address
being invalidated.  This series provide support for this feature.

When ARMv8.4-TTL is implemented, the operand for TLBIs looks like
below:

* +----------+-------+----------------------+
* |   ASID   |  TTL  |        BADDR         |
* +----------+-------+----------------------+
* |63      48|47   44|43                   0|


This version updates some codes implementation according to Peter's
suggestion, and adds some commit msg.

See patches for details, Thanks.

--
ChangeList:
v3:
minor changes: reduce the indentation levels of __tlbi_level().

v2:
rebase series on Linux 5.7-rc1 and simplify the code implementation.

v1:
add support for TTL feature in arm64.


Marc Zyngier (2):
  arm64: Detect the ARMv8.4 TTL feature
  arm64: Add level-hinted TLB invalidation helper

Peter Zijlstra (Intel) (1):
  tlb: mmu_gather: add tlb_flush_*_range APIs

Zhenyu Ye (3):
  arm64: Add tlbi_user_level TLB invalidation helper
  mm: tlb: Provide flush_*_tlb_range wrappers
  arm64: tlb: Set the TTL field in flush_tlb_range

 arch/arm64/include/asm/cpucaps.h  |  3 +-
 arch/arm64/include/asm/sysreg.h   |  1 +
 arch/arm64/include/asm/tlb.h      | 29 +++++++++++++++-
 arch/arm64/include/asm/tlbflush.h | 49 +++++++++++++++++++++++----
 arch/arm64/kernel/cpufeature.c    | 11 +++++++
 include/asm-generic/pgtable.h     | 12 +++++--
 include/asm-generic/tlb.h         | 55 ++++++++++++++++++++++---------
 mm/pgtable-generic.c              | 22 +++++++++++++
 8 files changed, 157 insertions(+), 25 deletions(-)

-- 
2.19.1



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

^ permalink raw reply

* [PATCH v3 4/6] tlb: mmu_gather: add tlb_flush_*_range APIs
From: Zhenyu Ye @ 2020-05-25 12:52 UTC (permalink / raw)
  To: catalin.marinas, peterz, mark.rutland, will, aneesh.kumar, akpm,
	npiggin, arnd, rostedt, maz, suzuki.poulose, tglx, yuzhao,
	Dave.Martin, steven.price, broonie, guohanjun
  Cc: linux-arch, yezhenyu2, linux-kernel, xiexiangyou, zhangshaokun,
	linux-mm, arm, prime.zeng, kuhn.chenqun, linux-arm-kernel
In-Reply-To: <20200525125300.794-1-yezhenyu2@huawei.com>

From: "Peter Zijlstra (Intel)" <peterz@infradead.org>

tlb_flush_{pte|pmd|pud|p4d}_range() adjust the tlb->start and
tlb->end, then set corresponding cleared_*.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 include/asm-generic/tlb.h | 55 ++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 15 deletions(-)

diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index 3f1649a8cf55..ef75ec86f865 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -512,6 +512,38 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm
 }
 #endif
 
+/*
+ * tlb_flush_{pte|pmd|pud|p4d}_range() adjust the tlb->start and tlb->end,
+ * and set corresponding cleared_*.
+ */
+static inline void tlb_flush_pte_range(struct mmu_gather *tlb,
+				     unsigned long address, unsigned long size)
+{
+	__tlb_adjust_range(tlb, address, size);
+	tlb->cleared_ptes = 1;
+}
+
+static inline void tlb_flush_pmd_range(struct mmu_gather *tlb,
+				     unsigned long address, unsigned long size)
+{
+	__tlb_adjust_range(tlb, address, size);
+	tlb->cleared_pmds = 1;
+}
+
+static inline void tlb_flush_pud_range(struct mmu_gather *tlb,
+				     unsigned long address, unsigned long size)
+{
+	__tlb_adjust_range(tlb, address, size);
+	tlb->cleared_puds = 1;
+}
+
+static inline void tlb_flush_p4d_range(struct mmu_gather *tlb,
+				     unsigned long address, unsigned long size)
+{
+	__tlb_adjust_range(tlb, address, size);
+	tlb->cleared_p4ds = 1;
+}
+
 #ifndef __tlb_remove_tlb_entry
 #define __tlb_remove_tlb_entry(tlb, ptep, address) do { } while (0)
 #endif
@@ -525,19 +557,17 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm
  */
 #define tlb_remove_tlb_entry(tlb, ptep, address)		\
 	do {							\
-		__tlb_adjust_range(tlb, address, PAGE_SIZE);	\
-		tlb->cleared_ptes = 1;				\
+		tlb_flush_pte_range(tlb, address, PAGE_SIZE);	\
 		__tlb_remove_tlb_entry(tlb, ptep, address);	\
 	} while (0)
 
 #define tlb_remove_huge_tlb_entry(h, tlb, ptep, address)	\
 	do {							\
 		unsigned long _sz = huge_page_size(h);		\
-		__tlb_adjust_range(tlb, address, _sz);		\
 		if (_sz == PMD_SIZE)				\
-			tlb->cleared_pmds = 1;			\
+			tlb_flush_pmd_range(tlb, address, _sz);	\
 		else if (_sz == PUD_SIZE)			\
-			tlb->cleared_puds = 1;			\
+			tlb_flush_pud_range(tlb, address, _sz);	\
 		__tlb_remove_tlb_entry(tlb, ptep, address);	\
 	} while (0)
 
@@ -551,8 +581,7 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm
 
 #define tlb_remove_pmd_tlb_entry(tlb, pmdp, address)			\
 	do {								\
-		__tlb_adjust_range(tlb, address, HPAGE_PMD_SIZE);	\
-		tlb->cleared_pmds = 1;					\
+		tlb_flush_pmd_range(tlb, address, HPAGE_PMD_SIZE);	\
 		__tlb_remove_pmd_tlb_entry(tlb, pmdp, address);		\
 	} while (0)
 
@@ -566,8 +595,7 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm
 
 #define tlb_remove_pud_tlb_entry(tlb, pudp, address)			\
 	do {								\
-		__tlb_adjust_range(tlb, address, HPAGE_PUD_SIZE);	\
-		tlb->cleared_puds = 1;					\
+		tlb_flush_pud_range(tlb, address, HPAGE_PUD_SIZE);	\
 		__tlb_remove_pud_tlb_entry(tlb, pudp, address);		\
 	} while (0)
 
@@ -592,9 +620,8 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm
 #ifndef pte_free_tlb
 #define pte_free_tlb(tlb, ptep, address)			\
 	do {							\
-		__tlb_adjust_range(tlb, address, PAGE_SIZE);	\
+		tlb_flush_pmd_range(tlb, address, PAGE_SIZE);	\
 		tlb->freed_tables = 1;				\
-		tlb->cleared_pmds = 1;				\
 		__pte_free_tlb(tlb, ptep, address);		\
 	} while (0)
 #endif
@@ -602,9 +629,8 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm
 #ifndef pmd_free_tlb
 #define pmd_free_tlb(tlb, pmdp, address)			\
 	do {							\
-		__tlb_adjust_range(tlb, address, PAGE_SIZE);	\
+		tlb_flush_pud_range(tlb, address, PAGE_SIZE);	\
 		tlb->freed_tables = 1;				\
-		tlb->cleared_puds = 1;				\
 		__pmd_free_tlb(tlb, pmdp, address);		\
 	} while (0)
 #endif
@@ -612,9 +638,8 @@ static inline void tlb_end_vma(struct mmu_gather *tlb, struct vm_area_struct *vm
 #ifndef pud_free_tlb
 #define pud_free_tlb(tlb, pudp, address)			\
 	do {							\
-		__tlb_adjust_range(tlb, address, PAGE_SIZE);	\
+		tlb_flush_p4d_range(tlb, address, PAGE_SIZE);	\
 		tlb->freed_tables = 1;				\
-		tlb->cleared_p4ds = 1;				\
 		__pud_free_tlb(tlb, pudp, address);		\
 	} while (0)
 #endif
-- 
2.19.1



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

^ permalink raw reply related

* [PATCH v3 5/6] mm: tlb: Provide flush_*_tlb_range wrappers
From: Zhenyu Ye @ 2020-05-25 12:52 UTC (permalink / raw)
  To: catalin.marinas, peterz, mark.rutland, will, aneesh.kumar, akpm,
	npiggin, arnd, rostedt, maz, suzuki.poulose, tglx, yuzhao,
	Dave.Martin, steven.price, broonie, guohanjun
  Cc: linux-arch, yezhenyu2, linux-kernel, xiexiangyou, zhangshaokun,
	linux-mm, arm, prime.zeng, kuhn.chenqun, linux-arm-kernel
In-Reply-To: <20200525125300.794-1-yezhenyu2@huawei.com>

This patch provides flush_{pte|pmd|pud|p4d}_tlb_range() in generic
code, which are expressed through the mmu_gather APIs.  These
interface set tlb->cleared_* and finally call tlb_flush(), so we
can do the tlb invalidation according to the information in
struct mmu_gather.

Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com>
---
 include/asm-generic/pgtable.h | 12 ++++++++++--
 mm/pgtable-generic.c          | 22 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 329b8c8ca703..8c92122ded9b 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -1161,11 +1161,19 @@ static inline int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
  * invalidate the entire TLB which is not desitable.
  * e.g. see arch/arc: flush_pmd_tlb_range
  */
-#define flush_pmd_tlb_range(vma, addr, end)	flush_tlb_range(vma, addr, end)
-#define flush_pud_tlb_range(vma, addr, end)	flush_tlb_range(vma, addr, end)
+extern void flush_pte_tlb_range(struct vm_area_struct *vma,
+				unsigned long addr, unsigned long end);
+extern void flush_pmd_tlb_range(struct vm_area_struct *vma,
+				unsigned long addr, unsigned long end);
+extern void flush_pud_tlb_range(struct vm_area_struct *vma,
+				unsigned long addr, unsigned long end);
+extern void flush_p4d_tlb_range(struct vm_area_struct *vma,
+				unsigned long addr, unsigned long end);
 #else
+#define flush_pte_tlb_range(vma, addr, end)	BUILD_BUG()
 #define flush_pmd_tlb_range(vma, addr, end)	BUILD_BUG()
 #define flush_pud_tlb_range(vma, addr, end)	BUILD_BUG()
+#define flush_p4d_tlb_range(vma, addr, end)	BUILD_BUG()
 #endif
 #endif
 
diff --git a/mm/pgtable-generic.c b/mm/pgtable-generic.c
index 3d7c01e76efc..3eff199d3507 100644
--- a/mm/pgtable-generic.c
+++ b/mm/pgtable-generic.c
@@ -101,6 +101,28 @@ pte_t ptep_clear_flush(struct vm_area_struct *vma, unsigned long address,
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 
+#ifndef __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
+
+#define FLUSH_Pxx_TLB_RANGE(_pxx)					\
+void flush_##_pxx##_tlb_range(struct vm_area_struct *vma,		\
+			      unsigned long addr, unsigned long end)	\
+{									\
+		struct mmu_gather tlb;					\
+									\
+		tlb_gather_mmu(&tlb, vma->vm_mm, addr, end);		\
+		tlb_start_vma(&tlb, vma);				\
+		tlb_flush_##_pxx##_range(&tlb, addr, end - addr);	\
+		tlb_end_vma(&tlb, vma);					\
+		tlb_finish_mmu(&tlb, addr, end);			\
+}
+
+FLUSH_Pxx_TLB_RANGE(pte)
+FLUSH_Pxx_TLB_RANGE(pmd)
+FLUSH_Pxx_TLB_RANGE(pud)
+FLUSH_Pxx_TLB_RANGE(p4d)
+
+#endif /* __HAVE_ARCH_FLUSH_PMD_TLB_RANGE */
+
 #ifndef __HAVE_ARCH_PMDP_SET_ACCESS_FLAGS
 int pmdp_set_access_flags(struct vm_area_struct *vma,
 			  unsigned long address, pmd_t *pmdp,
-- 
2.19.1



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

^ permalink raw reply related


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