Re: [PATCH v3 char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode for stimer0
On Sun, Jan 28, 2018 at 09:06:48PM +, Michael Kelley (EOSG) wrote: > From: Michael Kelley > > The 2016 version of Hyper-V offers the option to operate the guest VM > per-vcpu stimer's in Direct Mode, which means the timer interupts on its > own vector rather than queueing a VMbus message. Direct Mode reduces > timer processing overhead in both the hypervisor and the guest, and > avoids having timer interrupts pollute the VMbus interrupt stream for > the synthetic NIC and storage. This patch enables Direct Mode by > default on stimer0 when running on a version of Hyper-V that supports it, > with a hv_vmbus module parameter for disabling Direct Mode and reverting > to the old behavior. You didn't change the text here as there is no more module parameter :( > --- a/drivers/hv/hv.c > +++ b/drivers/hv/hv.c > @@ -27,8 +27,9 @@ > #include > #include > #include > -#include > +#include > #include > +#include Still need this .h file? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtlwifi: remove redundant initialization of 'cfg_cmd'
On Fri, 2018-01-26 at 13:52 +, Colin King wrote:
> From: Colin Ian King
>
> The initialization of cfg_cmd is redundant as the value is never read
> and it is being re-assigned to cfg_cmd = pwrcfgcmd[ary_idx] inside a
> loop, hence it can be removed.
>
> Cleans up clang warning:
> drivers/staging/rtlwifi/core.c:1819:22: warning: Value stored to
> 'cfg_cmd' during its initialization is never read
>
> Signed-off-by: Colin Ian King
It looks good to me.
Acked-by: Ping-Ke Shih
> ---
> drivers/staging/rtlwifi/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtlwifi/core.c b/drivers/staging/rtlwifi/core.c
> index a43d37452e8b..3ec039498208 100644
> --- a/drivers/staging/rtlwifi/core.c
> +++ b/drivers/staging/rtlwifi/core.c
> @@ -1816,7 +1816,7 @@ bool rtl_hal_pwrseqcmdparsing(struct rtl_priv *rtlpriv,
> u8 cut_version,
> u8 faversion, u8 interface_type,
> struct wlan_pwr_cfg pwrcfgcmd[])
> {
> - struct wlan_pwr_cfg cfg_cmd = {0};
> + struct wlan_pwr_cfg cfg_cmd;
> bool polling_bit = false;
> u32 ary_idx = 0;
> u8 value = 0;
> --
> 2.15.1
>
>
> --Please consider the environment before printing this e-mail.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode for stimer0
From: Michael Kelley
The 2016 version of Hyper-V offers the option to operate the guest VM
per-vcpu stimer's in Direct Mode, which means the timer interupts on its
own vector rather than queueing a VMbus message. Direct Mode reduces
timer processing overhead in both the hypervisor and the guest, and
avoids having timer interrupts pollute the VMbus interrupt stream for
the synthetic NIC and storage. This patch enables Direct Mode by
default on stimer0 when running on a version of Hyper-V that supports it,
with a hv_vmbus module parameter for disabling Direct Mode and reverting
to the old behavior.
In prep for coming support of Hyper-V on ARM64, the arch independent
portion of the code contains calls to routines that will be populated
on ARM64 but are not needed and do nothing on x86.
Signed-off-by: Michael Kelley
---
Changes since v1:
* Major rework to allocate and use a fixed interrupt vector
* Fixed block comment style
* Removed minor changes unrelated to Direct Mode
Changes since v2:
* Remove module parameter in drivers/hv/hv.c [Greg KH]
* Add declaration for hv_stimer0_vector_handler in mshyperv.h
---
arch/x86/entry/entry_32.S | 3 ++
arch/x86/entry/entry_64.S | 2 ++
arch/x86/include/asm/hardirq.h | 3 ++
arch/x86/include/asm/irq_vectors.h | 7 -
arch/x86/include/asm/mshyperv.h| 13
arch/x86/include/uapi/asm/hyperv.h | 3 ++
arch/x86/kernel/cpu/mshyperv.c | 41 -
arch/x86/kernel/irq.c | 9 ++
drivers/hv/hv.c| 62 --
drivers/hv/hyperv_vmbus.h | 4 ++-
10 files changed, 142 insertions(+), 5 deletions(-)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index ace8f32..1814991 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -882,6 +882,9 @@ BUILD_INTERRUPT3(xen_hvm_callback_vector,
HYPERVISOR_CALLBACK_VECTOR,
BUILD_INTERRUPT3(hyperv_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
hyperv_vector_handler)
+BUILD_INTERRUPT3(hv_stimer0_callback_vector, HYPERV_STIMER0_VECTOR,
+hv_stimer0_vector_handler)
+
#endif /* CONFIG_HYPERV */
ENTRY(page_fault)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index f048e38..23afac9 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1227,6 +1227,8 @@ apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \
#if IS_ENABLED(CONFIG_HYPERV)
apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \
hyperv_callback_vector hyperv_vector_handler
+apicinterrupt3 HYPERV_STIMER0_VECTOR \
+ hv_stimer0_callback_vector hv_stimer0_vector_handler
#endif /* CONFIG_HYPERV */
idtentry debug do_debughas_error_code=0
paranoid=1 shift_ist=DEBUG_STACK
diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index 51cc979..c788343 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -38,6 +38,9 @@
#if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
unsigned int irq_hv_callback_count;
#endif
+#if IS_ENABLED(CONFIG_HYPERV)
+ unsigned int hyperv_stimer0_count;
+#endif
} cacheline_aligned irq_cpustat_t;
DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
diff --git a/arch/x86/include/asm/irq_vectors.h
b/arch/x86/include/asm/irq_vectors.h
index 67421f6..6accf0b 100644
--- a/arch/x86/include/asm/irq_vectors.h
+++ b/arch/x86/include/asm/irq_vectors.h
@@ -103,7 +103,12 @@
#endif
#define MANAGED_IRQ_SHUTDOWN_VECTOR0xef
-#define LOCAL_TIMER_VECTOR 0xee
+
+#if IS_ENABLED(CONFIG_HYPERV)
+#define HYPERV_STIMER0_VECTOR 0xee
+#endif
+
+#define LOCAL_TIMER_VECTOR 0xed
#define NR_VECTORS 256
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index b623a42..5a170e5 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -171,6 +171,19 @@ static inline void vmbus_signal_eom(struct hv_message
*msg, u32 old_msg_type)
void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
void hv_remove_crash_handler(void);
+/*
+ * Routines for stimer0 Direct Mode handling.
+ * On x86/x64, there are no percpu actions to take.
+ */
+void hv_stimer0_vector_handler(struct pt_regs *regs);
+void hv_stimer0_callback_vector(void);
+int hv_setup_stimer0_irq(int *irq, int *vector, void (*handler)(void));
+void hv_remove_stimer0_irq(int irq);
+
+static inline void hv_enable_stimer0_percpu_irq(int irq) {}
+static inline void hv_disable_stimer0_percpu_irq(int irq) {}
+
+
#if IS_ENABLED(CONFIG_HYPERV)
extern struct clocksource *hyperv_cs;
extern void *hv_hypercall_pg;
diff --git a/arch/x86/include/uapi/asm/hyperv.h
b/arch/x86/include/uapi/asm/hyperv.h
index 1a5bfea..7213cb8 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -74,6 +74,9 @@
/* Crash MSR available */
#d
Re: [PATCH] staging: rtlwifi: add identifier names to function definition arguments
On 01/28/2018 08:05 AM, Erik Liodden wrote:
Add identifier names to function definition arguments to comply with
the kernel coding style and the naming convention in the rest of the
file.
Issues found by checkpatch.
Signed-off-by: Erik Liodden
---
As far as I am concerned, this patch is merely churning the source. Those
prototypes correctly describe the calling sequence to the compiler, which is all
that is necessary. Checkpatch is a moving specification. These were NOT
ERRORS/WARNINGS/CHECKS when this code was first introduced. Why should it be
made to conform to it now? Besides, you should use checkpatch in an advisory role.
NACK.
Larry
drivers/staging/rtlwifi/wifi.h | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/rtlwifi/wifi.h b/drivers/staging/rtlwifi/wifi.h
index ca0243f..a23bb17 100644
--- a/drivers/staging/rtlwifi/wifi.h
+++ b/drivers/staging/rtlwifi/wifi.h
@@ -2379,16 +2379,17 @@ struct rtl_hal_usbint_cfg {
u32 rx_max_size;
/* op - rx */
- void (*usb_rx_hdl)(struct ieee80211_hw *, struct sk_buff *);
- void (*usb_rx_segregate_hdl)(struct ieee80211_hw *, struct sk_buff *,
-struct sk_buff_head *);
+ void (*usb_rx_hdl)(struct ieee80211_hw *hw, struct sk_buff *skb);
+ void (*usb_rx_segregate_hdl)(struct ieee80211_hw *hw,
+struct sk_buff *skb,
+struct sk_buff_head *skbh);
/* tx */
- void (*usb_tx_cleanup)(struct ieee80211_hw *, struct sk_buff *);
- int (*usb_tx_post_hdl)(struct ieee80211_hw *, struct urb *,
- struct sk_buff *);
- struct sk_buff *(*usb_tx_aggregate_hdl)(struct ieee80211_hw *,
- struct sk_buff_head *);
+ void (*usb_tx_cleanup)(struct ieee80211_hw *hw, struct sk_buff *skb);
+ int (*usb_tx_post_hdl)(struct ieee80211_hw *hw, struct urb *urb,
+ struct sk_buff *skb);
+ struct sk_buff *(*usb_tx_aggregate_hdl)(struct ieee80211_hw *hw,
+ struct sk_buff_head *skbh);
/* endpoint mapping */
int (*usb_endpoint_mapping)(struct ieee80211_hw *hw);
@@ -2693,12 +2694,12 @@ struct rtl_btc_ops {
};
struct rtl_halmac_ops {
- int (*halmac_init_adapter)(struct rtl_priv *);
- int (*halmac_deinit_adapter)(struct rtl_priv *);
- int (*halmac_init_hal)(struct rtl_priv *);
- int (*halmac_deinit_hal)(struct rtl_priv *);
- int (*halmac_poweron)(struct rtl_priv *);
- int (*halmac_poweroff)(struct rtl_priv *);
+ int (*halmac_init_adapter)(struct rtl_priv *rtlpriv);
+ int (*halmac_deinit_adapter)(struct rtl_priv *rtlpriv);
+ int (*halmac_init_hal)(struct rtl_priv *rtlpriv);
+ int (*halmac_deinit_hal)(struct rtl_priv *rtlpriv);
+ int (*halmac_poweron)(struct rtl_priv *rtlpriv);
+ int (*halmac_poweroff)(struct rtl_priv *rtlpriv);
int (*halmac_phy_power_switch)(struct rtl_priv *rtlpriv, u8 enable);
int (*halmac_set_mac_address)(struct rtl_priv *rtlpriv, u8 hwport,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
staging: ion: ION allocation fall back order depends on heap linkage order
Hi,
According to my understanding, the allocation fall back order
completely depends on heap->id that is assigned during the heap
creation:
plist_for_each_entry(heap, &dev->heaps, node) {
/* if the caller didn't specify this heap id */
if (!((1 << heap->id) & heap_id_mask))
continue;
buffer = ion_buffer_create(heap, dev, len, flags);
if (!IS_ERR(buffer))
break;
}
On creation, each heap is added to the priority list according to the
priority assigned:
...
static int heap_id;
...
void ion_device_add_heap(struct ion_heap *heap)
{
...
heap->id = heap_id++;
...
}
The order of creation is the order of linkage defined in the Makefile.
Thus, by default, we have:
heap id 2, type ION_HEAP_TYPE_DMA
heap id 1, type ION_HEAP_TYPE_SYSTEM
heap id 0, type ION_HEAP_TYPE_SYSTEM_CONTIG
Changing the linkage order:
diff --git a/drivers/staging/android/ion/Makefile
b/drivers/staging/android/ion/Makefile
index bb30bf8..e05052c 100644
--- a/drivers/staging/android/ion/Makefile
+++ b/drivers/staging/android/ion/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_ION) += ion.o ion-ioctl.o ion_heap.o
-obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o
obj-$(CONFIG_ION_CARVEOUT_HEAP) += ion_carveout_heap.o
obj-$(CONFIG_ION_CHUNK_HEAP) += ion_chunk_heap.o
obj-$(CONFIG_ION_CMA_HEAP) += ion_cma_heap.o
+obj-$(CONFIG_ION_SYSTEM_HEAP) += ion_system_heap.o ion_page_pool.o
I get the following order:
heap id 2, type ION_HEAP_TYPE_SYSTEM
heap id 1, type ION_HEAP_TYPE_SYSTEM_CONTIG
heap id 0, type ION_HEAP_TYPE_DMA
So, if the user specifies more than 1 heap in the heap_id_mask during
allocation, the allocation fall back order completely depends on the
order of linkage. Probably, it's better to let the user to define the
fall back order (and NOT to be dependent on the linkage order at all)
?
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtlwifi: add identifier names to function definition arguments
Add identifier names to function definition arguments to comply with
the kernel coding style and the naming convention in the rest of the
file.
Issues found by checkpatch.
Signed-off-by: Erik Liodden
---
drivers/staging/rtlwifi/wifi.h | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/rtlwifi/wifi.h b/drivers/staging/rtlwifi/wifi.h
index ca0243f..a23bb17 100644
--- a/drivers/staging/rtlwifi/wifi.h
+++ b/drivers/staging/rtlwifi/wifi.h
@@ -2379,16 +2379,17 @@ struct rtl_hal_usbint_cfg {
u32 rx_max_size;
/* op - rx */
- void (*usb_rx_hdl)(struct ieee80211_hw *, struct sk_buff *);
- void (*usb_rx_segregate_hdl)(struct ieee80211_hw *, struct sk_buff *,
-struct sk_buff_head *);
+ void (*usb_rx_hdl)(struct ieee80211_hw *hw, struct sk_buff *skb);
+ void (*usb_rx_segregate_hdl)(struct ieee80211_hw *hw,
+struct sk_buff *skb,
+struct sk_buff_head *skbh);
/* tx */
- void (*usb_tx_cleanup)(struct ieee80211_hw *, struct sk_buff *);
- int (*usb_tx_post_hdl)(struct ieee80211_hw *, struct urb *,
- struct sk_buff *);
- struct sk_buff *(*usb_tx_aggregate_hdl)(struct ieee80211_hw *,
- struct sk_buff_head *);
+ void (*usb_tx_cleanup)(struct ieee80211_hw *hw, struct sk_buff *skb);
+ int (*usb_tx_post_hdl)(struct ieee80211_hw *hw, struct urb *urb,
+ struct sk_buff *skb);
+ struct sk_buff *(*usb_tx_aggregate_hdl)(struct ieee80211_hw *hw,
+ struct sk_buff_head *skbh);
/* endpoint mapping */
int (*usb_endpoint_mapping)(struct ieee80211_hw *hw);
@@ -2693,12 +2694,12 @@ struct rtl_btc_ops {
};
struct rtl_halmac_ops {
- int (*halmac_init_adapter)(struct rtl_priv *);
- int (*halmac_deinit_adapter)(struct rtl_priv *);
- int (*halmac_init_hal)(struct rtl_priv *);
- int (*halmac_deinit_hal)(struct rtl_priv *);
- int (*halmac_poweron)(struct rtl_priv *);
- int (*halmac_poweroff)(struct rtl_priv *);
+ int (*halmac_init_adapter)(struct rtl_priv *rtlpriv);
+ int (*halmac_deinit_adapter)(struct rtl_priv *rtlpriv);
+ int (*halmac_init_hal)(struct rtl_priv *rtlpriv);
+ int (*halmac_deinit_hal)(struct rtl_priv *rtlpriv);
+ int (*halmac_poweron)(struct rtl_priv *rtlpriv);
+ int (*halmac_poweroff)(struct rtl_priv *rtlpriv);
int (*halmac_phy_power_switch)(struct rtl_priv *rtlpriv, u8 enable);
int (*halmac_set_mac_address)(struct rtl_priv *rtlpriv, u8 hwport,
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] Staging: iio: ade7758: expand buf_lock to cover both buffer and state protection
On Sun, 2018-01-28 at 08:31 +, Jonathan Cameron wrote:
> On Thu, 25 Jan 2018 22:10:08 +0530
> Shreeya Patel wrote:
>
> >
> > iio_dev->mlock is to be used only by the IIO core for protecting
> > device mode changes between INDIO_DIRECT and INDIO_BUFFER.
> >
> > This patch replaces the use of mlock with the already established
> > buf_lock mutex.
> >
> > Introducing 'unlocked' __ade7758_spi_write_reg_8 and
> > __ade7758_spi_read_reg_8 functions to be used by
> > ade7758_write_samp_freq
> > and ade7758_read_samp_freq which avoids nested locks and maintains
> > atomicity between bus and device frequency changes.
> >
> > Signed-off-by: Shreeya Patel
> Hi Shreeya,
>
> This is now technically correct which is great.
> I would make one minor change to make it slightly easier to read.
>
> The read / write frequency functions now require the buf_lock to
> be held. That's not obvious so I would avoid this but moving
> the locking inside the functions where it is then clear that
> they are taking the unlocked forms of the register read/ write.
>
> This would also then make it clear why the normal locked form
> of _read_reg_8 is fine in the read_freq case but not the
> write_freq case. (Hence just use the normal locked form
> for the read and don't explicitly take the locks when
> reading the frequency - leave it to the register read function)
>
Hi,
I have introduced unlocked form of the _read_reg_8 also, which is wrong
on my side. I should have discarded the mlocks in the read_freq case
first, then there would have been no need of having unlocked form of
the _read_reg_8 :(
Please discard this patch and I'll send two patches in which first I'll
remove the mlocks from the read case and then I'll have only the
unlocked form of the _write_reg_8. In this way, there won't be any
useless code in the file for the read case.
Sorry, it took me a bit more time to understand.
Thanks
> Thanks,
>
> Jonathan
> >
> > ---
> >
> > Changes in v2
> > -Add static keyword to newly introduced functions and remove some
> > added comments which are not required.
> >
> >
> > drivers/staging/iio/meter/ade7758.h | 2 +-
> > drivers/staging/iio/meter/ade7758_core.c | 47
> > +++-
> > 2 files changed, 35 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/staging/iio/meter/ade7758.h
> > b/drivers/staging/iio/meter/ade7758.h
> > index 6ae78d8..2de81b5 100644
> > --- a/drivers/staging/iio/meter/ade7758.h
> > +++ b/drivers/staging/iio/meter/ade7758.h
> > @@ -111,7 +111,7 @@
> > * @trig: data ready trigger registered with iio
> > * @tx:transmit buffer
> > * @rx:receive buffer
> > - * @buf_lock: mutex to protect tx and rx
> > + * @buf_lock: mutex to protect tx, rx, read and
> > write frequency
> > **/
> > struct ade7758_state {
> > struct spi_device *us;
> > diff --git a/drivers/staging/iio/meter/ade7758_core.c
> > b/drivers/staging/iio/meter/ade7758_core.c
> > index 7b7ffe5..fed4684 100644
> > --- a/drivers/staging/iio/meter/ade7758_core.c
> > +++ b/drivers/staging/iio/meter/ade7758_core.c
> > @@ -24,17 +24,25 @@
> > #include "meter.h"
> > #include "ade7758.h"
> >
> > -int ade7758_spi_write_reg_8(struct device *dev, u8 reg_address, u8
> > val)
> > +static int __ade7758_spi_write_reg_8(struct device *dev, u8
> > reg_address, u8 val)
> > {
> > - int ret;
> > struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > struct ade7758_state *st = iio_priv(indio_dev);
> >
> > - mutex_lock(&st->buf_lock);
> > st->tx[0] = ADE7758_WRITE_REG(reg_address);
> > st->tx[1] = val;
> >
> > - ret = spi_write(st->us, st->tx, 2);
> > + return spi_write(st->us, st->tx, 2);
> > +}
> > +
> > +int ade7758_spi_write_reg_8(struct device *dev, u8 reg_address, u8
> > val)
> > +{
> > + int ret;
> > + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > + struct ade7758_state *st = iio_priv(indio_dev);
> > +
> > + mutex_lock(&st->buf_lock);
> > + ret = __ade7758_spi_write_reg_8(dev, reg_address, val);
> > mutex_unlock(&st->buf_lock);
> >
> > return ret;
> > @@ -91,7 +99,7 @@ static int ade7758_spi_write_reg_24(struct device
> > *dev, u8 reg_address,
> > return ret;
> > }
> >
> > -int ade7758_spi_read_reg_8(struct device *dev, u8 reg_address, u8
> > *val)
> > +static int __ade7758_spi_read_reg_8(struct device *dev, u8
> > reg_address, u8 *val)
> > {
> > struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > struct ade7758_state *st = iio_priv(indio_dev);
> > @@ -111,7 +119,6 @@ int ade7758_spi_read_reg_8(struct device *dev,
> > u8 reg_address, u8 *val)
> > },
> > };
> >
> > - mutex_lock(&st->buf_lock);
> > st->tx[0] = ADE7758_READ_REG(reg_address);
> > st->tx[1] = 0;
> >
> > @@ -124,7 +131,19 @@ int ade7758_spi_read_reg_8(struct device *dev,
> > u8 reg_address, u8 *val)
> > *val = st->rx[0];
> >
> > error_ret:
> > + r
Re: [PATCH v2] Staging: iio: ade7758: expand buf_lock to cover both buffer and state protection
On Thu, 25 Jan 2018 22:10:08 +0530
Shreeya Patel wrote:
> iio_dev->mlock is to be used only by the IIO core for protecting
> device mode changes between INDIO_DIRECT and INDIO_BUFFER.
>
> This patch replaces the use of mlock with the already established
> buf_lock mutex.
>
> Introducing 'unlocked' __ade7758_spi_write_reg_8 and
> __ade7758_spi_read_reg_8 functions to be used by ade7758_write_samp_freq
> and ade7758_read_samp_freq which avoids nested locks and maintains
> atomicity between bus and device frequency changes.
>
> Signed-off-by: Shreeya Patel
Hi Shreeya,
This is now technically correct which is great.
I would make one minor change to make it slightly easier to read.
The read / write frequency functions now require the buf_lock to
be held. That's not obvious so I would avoid this but moving
the locking inside the functions where it is then clear that
they are taking the unlocked forms of the register read/ write.
This would also then make it clear why the normal locked form
of _read_reg_8 is fine in the read_freq case but not the
write_freq case. (Hence just use the normal locked form
for the read and don't explicitly take the locks when
reading the frequency - leave it to the register read function)
Thanks,
Jonathan
> ---
>
> Changes in v2
> -Add static keyword to newly introduced functions and remove some
> added comments which are not required.
>
>
> drivers/staging/iio/meter/ade7758.h | 2 +-
> drivers/staging/iio/meter/ade7758_core.c | 47
> +++-
> 2 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/iio/meter/ade7758.h
> b/drivers/staging/iio/meter/ade7758.h
> index 6ae78d8..2de81b5 100644
> --- a/drivers/staging/iio/meter/ade7758.h
> +++ b/drivers/staging/iio/meter/ade7758.h
> @@ -111,7 +111,7 @@
> * @trig:data ready trigger registered with iio
> * @tx: transmit buffer
> * @rx: receive buffer
> - * @buf_lock:mutex to protect tx and rx
> + * @buf_lock:mutex to protect tx, rx, read and write
> frequency
> **/
> struct ade7758_state {
> struct spi_device *us;
> diff --git a/drivers/staging/iio/meter/ade7758_core.c
> b/drivers/staging/iio/meter/ade7758_core.c
> index 7b7ffe5..fed4684 100644
> --- a/drivers/staging/iio/meter/ade7758_core.c
> +++ b/drivers/staging/iio/meter/ade7758_core.c
> @@ -24,17 +24,25 @@
> #include "meter.h"
> #include "ade7758.h"
>
> -int ade7758_spi_write_reg_8(struct device *dev, u8 reg_address, u8 val)
> +static int __ade7758_spi_write_reg_8(struct device *dev, u8 reg_address, u8
> val)
> {
> - int ret;
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct ade7758_state *st = iio_priv(indio_dev);
>
> - mutex_lock(&st->buf_lock);
> st->tx[0] = ADE7758_WRITE_REG(reg_address);
> st->tx[1] = val;
>
> - ret = spi_write(st->us, st->tx, 2);
> + return spi_write(st->us, st->tx, 2);
> +}
> +
> +int ade7758_spi_write_reg_8(struct device *dev, u8 reg_address, u8 val)
> +{
> + int ret;
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct ade7758_state *st = iio_priv(indio_dev);
> +
> + mutex_lock(&st->buf_lock);
> + ret = __ade7758_spi_write_reg_8(dev, reg_address, val);
> mutex_unlock(&st->buf_lock);
>
> return ret;
> @@ -91,7 +99,7 @@ static int ade7758_spi_write_reg_24(struct device *dev, u8
> reg_address,
> return ret;
> }
>
> -int ade7758_spi_read_reg_8(struct device *dev, u8 reg_address, u8 *val)
> +static int __ade7758_spi_read_reg_8(struct device *dev, u8 reg_address, u8
> *val)
> {
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct ade7758_state *st = iio_priv(indio_dev);
> @@ -111,7 +119,6 @@ int ade7758_spi_read_reg_8(struct device *dev, u8
> reg_address, u8 *val)
> },
> };
>
> - mutex_lock(&st->buf_lock);
> st->tx[0] = ADE7758_READ_REG(reg_address);
> st->tx[1] = 0;
>
> @@ -124,7 +131,19 @@ int ade7758_spi_read_reg_8(struct device *dev, u8
> reg_address, u8 *val)
> *val = st->rx[0];
>
> error_ret:
> + return ret;
> +}
> +
> +int ade7758_spi_read_reg_8(struct device *dev, u8 reg_address, u8 *val)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct ade7758_state *st = iio_priv(indio_dev);
> + int ret;
> +
> + mutex_lock(&st->buf_lock);
> + ret = __ade7758_spi_read_reg_8(dev, reg_address, val);
> mutex_unlock(&st->buf_lock);
> +
> return ret;
> }
>
> @@ -470,7 +489,7 @@ static int ade7758_read_samp_freq(struct device *dev, int
> *val)
> int ret;
> u8 t;
>
> - ret = ade7758_spi_read_reg_8(dev, ADE7758_WAVMODE, &t);
> + ret = __ade7758_spi_read_reg_8(dev, ADE7758_WAVMODE, &t);
> if (ret)
> return ret;
>
> @@ -503,14 +522,14 @@ static int ade7758_write_samp_freq(struct device *dev,
> int val)
>
