[alsa-devel] [PATCH v1 1/4] ASoC: Intel: common: Replace custom implementation of readq / writeq

The readq() and writeq() helpers are available in the linux/io-64-nonatomic-hi-lo.h and linux/io-64-nonatomic-lo-hi.h headers.
Replace custom implementation by the generic helpers.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- sound/soc/intel/common/sst-dsp.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c index 11c0805393ff..748f1f5c02df 100644 --- a/sound/soc/intel/common/sst-dsp.c +++ b/sound/soc/intel/common/sst-dsp.c @@ -22,6 +22,8 @@ #include <linux/io.h> #include <linux/delay.h>
+#include <linux/io-64-nonatomic-lo-hi.h> + #include "sst-dsp.h" #include "sst-dsp-priv.h"
@@ -43,16 +45,13 @@ EXPORT_SYMBOL_GPL(sst_shim32_read);
void sst_shim32_write64(void __iomem *addr, u32 offset, u64 value) { - memcpy_toio(addr + offset, &value, sizeof(value)); + lo_hi_writeq(value, addr + offset); } EXPORT_SYMBOL_GPL(sst_shim32_write64);
u64 sst_shim32_read64(void __iomem *addr, u32 offset) { - u64 val; - - memcpy_fromio(&val, addr + offset, sizeof(val)); - return val; + return lo_hi_readq(addr + offset); } EXPORT_SYMBOL_GPL(sst_shim32_read64);

The readq() and writeq() helpers are available in the linux/io-64-nonatomic-hi-lo.h and linux/io-64-nonatomic-lo-hi.h headers.
Replace custom implementation by the generic helpers.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- sound/soc/intel/atom/sst/sst.h | 1 - sound/soc/intel/atom/sst/sst_pvt.c | 19 +++++-------------- 2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/sound/soc/intel/atom/sst/sst.h b/sound/soc/intel/atom/sst/sst.h index 5c9a51cc77aa..c585b07925f8 100644 --- a/sound/soc/intel/atom/sst/sst.h +++ b/sound/soc/intel/atom/sst/sst.h @@ -536,7 +536,6 @@ void sst_add_to_dispatch_list_and_post(struct intel_sst_drv *sst, int sst_pm_runtime_put(struct intel_sst_drv *sst_drv); int sst_shim_write(void __iomem *addr, int offset, int value); u32 sst_shim_read(void __iomem *addr, int offset); -u64 sst_reg_read64(void __iomem *addr, int offset); int sst_shim_write64(void __iomem *addr, int offset, u64 value); u64 sst_shim_read64(void __iomem *addr, int offset); void sst_set_fw_state_locked( diff --git a/sound/soc/intel/atom/sst/sst_pvt.c b/sound/soc/intel/atom/sst/sst_pvt.c index b1e6b8f34a6a..2452cbd77033 100644 --- a/sound/soc/intel/atom/sst/sst_pvt.c +++ b/sound/soc/intel/atom/sst/sst_pvt.c @@ -26,12 +26,15 @@ #include <linux/pm_runtime.h> #include <linux/sched.h> #include <linux/delay.h> +#include <linux/io-64-nonatomic-lo-hi.h> #include <sound/asound.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/soc.h> #include <sound/compress_driver.h> + #include <asm/platform_sst_audio.h> + #include "../sst-mfld-platform.h" #include "sst.h" #include "../../common/sst-dsp.h" @@ -47,27 +50,15 @@ u32 sst_shim_read(void __iomem *addr, int offset) return readl(addr + offset); }
-u64 sst_reg_read64(void __iomem *addr, int offset) -{ - u64 val = 0; - - memcpy_fromio(&val, addr + offset, sizeof(val)); - - return val; -} - int sst_shim_write64(void __iomem *addr, int offset, u64 value) { - memcpy_toio(addr + offset, &value, sizeof(value)); + lo_hi_writeq(value, addr + offset); return 0; }
u64 sst_shim_read64(void __iomem *addr, int offset) { - u64 val = 0; - - memcpy_fromio(&val, addr + offset, sizeof(val)); - return val; + return lo_hi_readq(addr + offset); }
void sst_set_fw_state_locked(

There is no point to always return 0. Convert sst_shim_write() and sst_shim_write64() to return void.
No caller is checking for return value.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- sound/soc/intel/atom/sst/sst.h | 4 ++-- sound/soc/intel/atom/sst/sst_pvt.c | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/sound/soc/intel/atom/sst/sst.h b/sound/soc/intel/atom/sst/sst.h index c585b07925f8..f00c13d53d23 100644 --- a/sound/soc/intel/atom/sst/sst.h +++ b/sound/soc/intel/atom/sst/sst.h @@ -534,9 +534,9 @@ u32 relocate_imr_addr_mrfld(u32 base_addr); void sst_add_to_dispatch_list_and_post(struct intel_sst_drv *sst, struct ipc_post *msg); int sst_pm_runtime_put(struct intel_sst_drv *sst_drv); -int sst_shim_write(void __iomem *addr, int offset, int value); +void sst_shim_write(void __iomem *addr, int offset, int value); u32 sst_shim_read(void __iomem *addr, int offset); -int sst_shim_write64(void __iomem *addr, int offset, u64 value); +void sst_shim_write64(void __iomem *addr, int offset, u64 value); u64 sst_shim_read64(void __iomem *addr, int offset); void sst_set_fw_state_locked( struct intel_sst_drv *sst_drv_ctx, int sst_state); diff --git a/sound/soc/intel/atom/sst/sst_pvt.c b/sound/soc/intel/atom/sst/sst_pvt.c index 2452cbd77033..00d58f41231a 100644 --- a/sound/soc/intel/atom/sst/sst_pvt.c +++ b/sound/soc/intel/atom/sst/sst_pvt.c @@ -39,10 +39,9 @@ #include "sst.h" #include "../../common/sst-dsp.h"
-int sst_shim_write(void __iomem *addr, int offset, int value) +void sst_shim_write(void __iomem *addr, int offset, int value) { writel(value, addr + offset); - return 0; }
u32 sst_shim_read(void __iomem *addr, int offset) @@ -50,10 +49,9 @@ u32 sst_shim_read(void __iomem *addr, int offset) return readl(addr + offset); }
-int sst_shim_write64(void __iomem *addr, int offset, u64 value) +void sst_shim_write64(void __iomem *addr, int offset, u64 value) { lo_hi_writeq(value, addr + offset); - return 0; }
u64 sst_shim_read64(void __iomem *addr, int offset)

Snail mail addresses are subject to change. This already happened once, so, better to get rid of mention of such addresses for good.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- sound/soc/intel/boards/mfld_machine.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/sound/soc/intel/boards/mfld_machine.c b/sound/soc/intel/boards/mfld_machine.c index 4e08885f37aa..c9ad717a224e 100644 --- a/sound/soc/intel/boards/mfld_machine.c +++ b/sound/soc/intel/boards/mfld_machine.c @@ -1,5 +1,5 @@ /* - * mfld_machine.c - ASoc Machine driver for Intel Medfield MID platform + * ASoC Machine driver for Intel Medfield MID platform * * Copyright (C) 2010 Intel Corp * Author: Vinod Koul vinod.koul@intel.com @@ -15,10 +15,6 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. - * * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */

On Tue, 2017-01-31 at 16:14 +0200, Andy Shevchenko wrote:
The readq() and writeq() helpers are available in the linux/io-64-nonatomic-hi-lo.h and linux/io-64-nonatomic-lo-hi.h headers.
Replace custom implementation by the generic helpers.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
All
Acked-by: Liam Girdwood liam.r.girdwood@linux.intel.com

On Tue, Jan 31, 2017 at 04:14:22PM +0200, Andy Shevchenko wrote:
The readq() and writeq() helpers are available in the linux/io-64-nonatomic-hi-lo.h and linux/io-64-nonatomic-lo-hi.h headers.
Replace custom implementation by the generic helpers.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
sound/soc/intel/common/sst-dsp.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c index 11c0805393ff..748f1f5c02df 100644 --- a/sound/soc/intel/common/sst-dsp.c +++ b/sound/soc/intel/common/sst-dsp.c @@ -22,6 +22,8 @@ #include <linux/io.h> #include <linux/delay.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
#include "sst-dsp.h" #include "sst-dsp-priv.h"
@@ -43,16 +45,13 @@ EXPORT_SYMBOL_GPL(sst_shim32_read);
void sst_shim32_write64(void __iomem *addr, u32 offset, u64 value) {
- memcpy_toio(addr + offset, &value, sizeof(value));
- lo_hi_writeq(value, addr + offset);
why not use writeq here and for 32bit this becomes lo_hi_writeq(), or did I miss something here..
} EXPORT_SYMBOL_GPL(sst_shim32_write64);
u64 sst_shim32_read64(void __iomem *addr, u32 offset) {
- u64 val;
- memcpy_fromio(&val, addr + offset, sizeof(val));
- return val;
- return lo_hi_readq(addr + offset);
} EXPORT_SYMBOL_GPL(sst_shim32_read64);
-- 2.11.0

On Mon, 2017-02-06 at 22:33 +0530, Vinod Koul wrote:
On Tue, Jan 31, 2017 at 04:14:22PM +0200, Andy Shevchenko wrote:
The readq() and writeq() helpers are available in the linux/io-64-nonatomic-hi-lo.h and linux/io-64-nonatomic-lo-hi.h headers.
Replace custom implementation by the generic helpers.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
sound/soc/intel/common/sst-dsp.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c index 11c0805393ff..748f1f5c02df 100644 --- a/sound/soc/intel/common/sst-dsp.c +++ b/sound/soc/intel/common/sst-dsp.c @@ -22,6 +22,8 @@ #include <linux/io.h> #include <linux/delay.h> +#include <linux/io-64-nonatomic-lo-hi.h>
#include "sst-dsp.h" #include "sst-dsp-priv.h" @@ -43,16 +45,13 @@ EXPORT_SYMBOL_GPL(sst_shim32_read); void sst_shim32_write64(void __iomem *addr, u32 offset, u64 value) {
- memcpy_toio(addr + offset, &value, sizeof(value));
- lo_hi_writeq(value, addr + offset);
why not use writeq here and for 32bit this becomes lo_hi_writeq(), or did I miss something here..
I'm not sure our hardware will correctly handle writeq()/readq(). OCP bus which is quite likely used internally is 32-bit bus.
} EXPORT_SYMBOL_GPL(sst_shim32_write64); u64 sst_shim32_read64(void __iomem *addr, u32 offset) {
- u64 val;
- memcpy_fromio(&val, addr + offset, sizeof(val));
- return val;
- return lo_hi_readq(addr + offset);
} EXPORT_SYMBOL_GPL(sst_shim32_read64); -- 2.11.0

On Wed, Feb 08, 2017 at 01:59:35PM +0200, Andy Shevchenko wrote:
On Mon, 2017-02-06 at 22:33 +0530, Vinod Koul wrote:
On Tue, Jan 31, 2017 at 04:14:22PM +0200, Andy Shevchenko wrote:
The readq() and writeq() helpers are available in the linux/io-64-nonatomic-hi-lo.h and linux/io-64-nonatomic-lo-hi.h headers.
Replace custom implementation by the generic helpers.
Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
sound/soc/intel/common/sst-dsp.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c index 11c0805393ff..748f1f5c02df 100644 --- a/sound/soc/intel/common/sst-dsp.c +++ b/sound/soc/intel/common/sst-dsp.c @@ -22,6 +22,8 @@ #include <linux/io.h> #include <linux/delay.h> +#include <linux/io-64-nonatomic-lo-hi.h>
#include "sst-dsp.h" #include "sst-dsp-priv.h" @@ -43,16 +45,13 @@ EXPORT_SYMBOL_GPL(sst_shim32_read); void sst_shim32_write64(void __iomem *addr, u32 offset, u64 value) {
- memcpy_toio(addr + offset, &value, sizeof(value));
- lo_hi_writeq(value, addr + offset);
why not use writeq here and for 32bit this becomes lo_hi_writeq(), or did I miss something here..
I'm not sure our hardware will correctly handle writeq()/readq(). OCP bus which is quite likely used internally is 32-bit bus.
Yes it is a 32 bit bus, but then we have a bridge which splilts it up. So it should work, would be worth to test this and update.
participants (3)
-
Andy Shevchenko
-
Liam Girdwood
-
Vinod Koul