-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expand SoundWire MBQ register map support #5268
base: topic/sof-dev
Are you sure you want to change the base?
Changes from all commits
68a9d45
937d8c1
f540b3c
0be106f
2a9c06e
e4d6674
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,45 +1,187 @@ | ||
// SPDX-License-Identifier: GPL-2.0 | ||
// Copyright(c) 2020 Intel Corporation. | ||
|
||
#include <linux/bits.h> | ||
#include <linux/delay.h> | ||
#include <linux/device.h> | ||
#include <linux/errno.h> | ||
#include <linux/iopoll.h> | ||
#include <linux/module.h> | ||
#include <linux/regmap.h> | ||
#include <linux/soundwire/sdw.h> | ||
#include <linux/soundwire/sdw_registers.h> | ||
#include <sound/sdca_function.h> | ||
#include "internal.h" | ||
|
||
struct regmap_mbq_context { | ||
struct device *dev; | ||
|
||
struct regmap_sdw_mbq_cfg cfg; | ||
|
||
int val_size; | ||
bool (*readable_reg)(struct device *dev, unsigned int reg); | ||
}; | ||
|
||
static int regmap_sdw_mbq_size(struct regmap_mbq_context *ctx, unsigned int reg) | ||
{ | ||
int size = ctx->val_size; | ||
|
||
if (ctx->cfg.mbq_size) { | ||
size = ctx->cfg.mbq_size(ctx->dev, reg); | ||
if (!size || size > ctx->val_size) | ||
return -EINVAL; | ||
} | ||
|
||
return size; | ||
} | ||
|
||
static bool regmap_sdw_mbq_deferrable(struct regmap_mbq_context *ctx, unsigned int reg) | ||
{ | ||
if (ctx->cfg.deferrable) | ||
return ctx->cfg.deferrable(ctx->dev, reg); | ||
|
||
return false; | ||
} | ||
|
||
static int regmap_sdw_mbq_poll_busy(struct sdw_slave *slave, unsigned int reg, | ||
struct regmap_mbq_context *ctx) | ||
{ | ||
struct device *dev = &slave->dev; | ||
int val, ret = 0; | ||
|
||
dev_dbg(dev, "Deferring transaction for 0x%x\n", reg); | ||
|
||
reg = SDW_SDCA_CTL(SDW_SDCA_CTL_FUNC(reg), 0, | ||
SDCA_CTL_ENTITY_0_FUNCTION_STATUS, 0); | ||
|
||
if (ctx->readable_reg(dev, reg)) { | ||
ret = read_poll_timeout(sdw_read_no_pm, val, | ||
val < 0 || !(val & SDCA_CTL_ENTITY_0_FUNCTION_BUSY), | ||
ctx->cfg.timeout_us, ctx->cfg.retry_us, | ||
false, slave, reg); | ||
if (val < 0) | ||
return val; | ||
if (ret) | ||
dev_err(dev, "Function busy timed out 0x%x: %d\n", reg, val); | ||
} else { | ||
fsleep(ctx->cfg.timeout_us); | ||
} | ||
|
||
return ret; | ||
} | ||
|
||
static int regmap_sdw_mbq_write_impl(struct sdw_slave *slave, | ||
unsigned int reg, unsigned int val, | ||
int mbq_size, bool deferrable) | ||
{ | ||
int shift = mbq_size * BITS_PER_BYTE; | ||
int ret; | ||
|
||
while (--mbq_size > 0) { | ||
shift -= BITS_PER_BYTE; | ||
|
||
ret = sdw_write_no_pm(slave, SDW_SDCA_MBQ_CTL(reg), | ||
(val >> shift) & 0xff); | ||
if (ret < 0) | ||
return ret; | ||
} | ||
|
||
ret = sdw_write_no_pm(slave, reg, val & 0xff); | ||
if (deferrable && ret == -ENODATA) | ||
return -EAGAIN; | ||
|
||
return ret; | ||
} | ||
|
||
static int regmap_sdw_mbq_write(void *context, unsigned int reg, unsigned int val) | ||
{ | ||
struct device *dev = context; | ||
struct regmap_mbq_context *ctx = context; | ||
struct device *dev = ctx->dev; | ||
struct sdw_slave *slave = dev_to_sdw_dev(dev); | ||
bool deferrable = regmap_sdw_mbq_deferrable(ctx, reg); | ||
int mbq_size = regmap_sdw_mbq_size(ctx, reg); | ||
int ret; | ||
|
||
ret = sdw_write_no_pm(slave, SDW_SDCA_MBQ_CTL(reg), (val >> 8) & 0xff); | ||
if (ret < 0) | ||
return ret; | ||
if (mbq_size < 0) | ||
return mbq_size; | ||
|
||
/* | ||
* Technically the spec does allow a device to set itself to busy for | ||
* internal reasons, but since it doesn't provide any information on | ||
* how to handle timeouts in that case, for now the code will only | ||
* process a single wait/timeout on function busy and a single retry | ||
* of the transaction. | ||
*/ | ||
ret = regmap_sdw_mbq_write_impl(slave, reg, val, mbq_size, deferrable); | ||
if (ret == -EAGAIN) { | ||
ret = regmap_sdw_mbq_poll_busy(slave, reg, ctx); | ||
if (ret) | ||
return ret; | ||
|
||
ret = regmap_sdw_mbq_write_impl(slave, reg, val, mbq_size, false); | ||
} | ||
|
||
return ret; | ||
} | ||
|
||
static int regmap_sdw_mbq_read_impl(struct sdw_slave *slave, | ||
unsigned int reg, unsigned int *val, | ||
int mbq_size, bool deferrable) | ||
{ | ||
int shift = BITS_PER_BYTE; | ||
int read; | ||
|
||
read = sdw_read_no_pm(slave, reg); | ||
if (read < 0) { | ||
if (deferrable && read == -ENODATA) | ||
return -EAGAIN; | ||
|
||
return read; | ||
} | ||
|
||
*val = read; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, you add it in this commit. IMHO, this belongs to the |
||
|
||
while (--mbq_size > 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this ever be large and block userspace ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be fine in regmap_sdw_mbq_size we do:
And we don't allow val_size to get too big in regmap_sdw_mbq_config_check:
So the largest mbq_size can ever be is sizeof(unsigned int). |
||
read = sdw_read_no_pm(slave, SDW_SDCA_MBQ_CTL(reg)); | ||
if (read < 0) | ||
return read; | ||
|
||
*val |= read << shift; | ||
shift += BITS_PER_BYTE; | ||
} | ||
|
||
return sdw_write_no_pm(slave, reg, val & 0xff); | ||
return 0; | ||
} | ||
|
||
static int regmap_sdw_mbq_read(void *context, unsigned int reg, unsigned int *val) | ||
{ | ||
struct device *dev = context; | ||
struct regmap_mbq_context *ctx = context; | ||
struct device *dev = ctx->dev; | ||
struct sdw_slave *slave = dev_to_sdw_dev(dev); | ||
int read0; | ||
int read1; | ||
bool deferrable = regmap_sdw_mbq_deferrable(ctx, reg); | ||
int mbq_size = regmap_sdw_mbq_size(ctx, reg); | ||
int ret; | ||
|
||
read0 = sdw_read_no_pm(slave, reg); | ||
if (read0 < 0) | ||
return read0; | ||
if (mbq_size < 0) | ||
return mbq_size; | ||
|
||
read1 = sdw_read_no_pm(slave, SDW_SDCA_MBQ_CTL(reg)); | ||
if (read1 < 0) | ||
return read1; | ||
/* | ||
* Technically the spec does allow a device to set itself to busy for | ||
* internal reasons, but since it doesn't provide any information on | ||
* how to handle timeouts in that case, for now the code will only | ||
* process a single wait/timeout on function busy and a single retry | ||
* of the transaction. | ||
*/ | ||
ret = regmap_sdw_mbq_read_impl(slave, reg, val, mbq_size, deferrable); | ||
if (ret == -EAGAIN) { | ||
ret = regmap_sdw_mbq_poll_busy(slave, reg, ctx); | ||
if (ret) | ||
return ret; | ||
|
||
*val = (read1 << 8) | read0; | ||
ret = regmap_sdw_mbq_read_impl(slave, reg, val, mbq_size, false); | ||
} | ||
|
||
return 0; | ||
return ret; | ||
} | ||
|
||
static const struct regmap_bus regmap_sdw_mbq = { | ||
|
@@ -51,8 +193,7 @@ static const struct regmap_bus regmap_sdw_mbq = { | |
|
||
static int regmap_sdw_mbq_config_check(const struct regmap_config *config) | ||
{ | ||
/* MBQ-based controls are only 16-bits for now */ | ||
if (config->val_bits != 16) | ||
if (config->val_bits > (sizeof(unsigned int) * BITS_PER_BYTE)) | ||
return -ENOTSUPP; | ||
|
||
/* Registers are 32 bits wide */ | ||
|
@@ -65,35 +206,69 @@ static int regmap_sdw_mbq_config_check(const struct regmap_config *config) | |
return 0; | ||
} | ||
|
||
static struct regmap_mbq_context * | ||
regmap_sdw_mbq_gen_context(struct device *dev, | ||
const struct regmap_config *config, | ||
const struct regmap_sdw_mbq_cfg *mbq_config) | ||
{ | ||
struct regmap_mbq_context *ctx; | ||
|
||
ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); | ||
if (!ctx) | ||
return ERR_PTR(-ENOMEM); | ||
|
||
ctx->dev = dev; | ||
|
||
if (mbq_config) | ||
ctx->cfg = *mbq_config; | ||
|
||
ctx->val_size = config->val_bits / BITS_PER_BYTE; | ||
ctx->readable_reg = config->readable_reg; | ||
|
||
return ctx; | ||
} | ||
|
||
struct regmap *__regmap_init_sdw_mbq(struct sdw_slave *sdw, | ||
const struct regmap_config *config, | ||
const struct regmap_sdw_mbq_cfg *mbq_config, | ||
struct lock_class_key *lock_key, | ||
const char *lock_name) | ||
{ | ||
struct regmap_mbq_context *ctx; | ||
int ret; | ||
|
||
ret = regmap_sdw_mbq_config_check(config); | ||
if (ret) | ||
return ERR_PTR(ret); | ||
|
||
return __regmap_init(&sdw->dev, ®map_sdw_mbq, | ||
&sdw->dev, config, lock_key, lock_name); | ||
ctx = regmap_sdw_mbq_gen_context(&sdw->dev, config, mbq_config); | ||
if (IS_ERR(ctx)) | ||
return ERR_CAST(ctx); | ||
|
||
return __regmap_init(&sdw->dev, ®map_sdw_mbq, ctx, | ||
config, lock_key, lock_name); | ||
} | ||
EXPORT_SYMBOL_GPL(__regmap_init_sdw_mbq); | ||
|
||
struct regmap *__devm_regmap_init_sdw_mbq(struct sdw_slave *sdw, | ||
const struct regmap_config *config, | ||
const struct regmap_sdw_mbq_cfg *mbq_config, | ||
struct lock_class_key *lock_key, | ||
const char *lock_name) | ||
{ | ||
struct regmap_mbq_context *ctx; | ||
int ret; | ||
|
||
ret = regmap_sdw_mbq_config_check(config); | ||
if (ret) | ||
return ERR_PTR(ret); | ||
|
||
return __devm_regmap_init(&sdw->dev, ®map_sdw_mbq, | ||
&sdw->dev, config, lock_key, lock_name); | ||
ctx = regmap_sdw_mbq_gen_context(&sdw->dev, config, mbq_config); | ||
if (IS_ERR(ctx)) | ||
return ERR_CAST(ctx); | ||
|
||
return __devm_regmap_init(&sdw->dev, ®map_sdw_mbq, ctx, | ||
config, lock_key, lock_name); | ||
} | ||
EXPORT_SYMBOL_GPL(__devm_regmap_init_sdw_mbq); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need
*val = read;
after this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed yes, thank you very much for spotting that. I will get it fixed up and do a respin.