mirror of
https://github.com/torvalds/linux.git
synced 2026-07-28 10:09:10 +02:00
ASoC: tegra210: simplify ADX/AMX byte map get/put logic
Piyush Patle <piyushpatle228@gmail.com> says:
The Tegra210 ADX and AMX drivers both keep their "Byte Map N" ALSA
control state as a byte-packed u32 map[] array along with a separate
byte_mask[] bitmap. This is because the control range exposed to
userspace is [0, 256], where 256 is the "disabled" sentinel and
does not fit in a byte, so the two arrays have to be cross-checked
on every get()/put().
This series stores each slot as a u16 holding the user-visible
value directly, turning get_byte_map() into a direct return and
put_byte_map() into a compare-and-store. The hardware-facing packed
RAM word and the IN_BYTE_EN / OUT_BYTE_EN enable masks are computed
on the fly inside each write_map_ram() callback, which is the only
place that needs to know the hardware layout. The byte_mask[] field
is kept in the driver struct but allocated dynamically in probe()
using devm_kcalloc() with soc_data->byte_mask_size, and zeroed +
recomputed on each write_map_ram() call.
There is no userspace-visible ABI change. Control declarations,
ranges, initial values and handling of out-of-range writes is
preserved by treating values outside [0, 255] as disabled (256),
matching previous behavior. As a side effect each patch also fixes
a latent bug in put_byte_map() where an enabled-to-enabled value
change was not persisted.
The packed RAM word construction is also updated to ensure the shift
operates on a u32 value, avoiding potential undefined behavior due
to signed integer promotion.
Addresses TODO comments left in tegra210_{adx,amx}_get_byte_map().
This commit is contained in:
Mark Brown
commit
eb0d565514
|
|
@ -4,6 +4,7 @@
|
|||
//
|
||||
// tegra210_adx.c - Tegra210 ADX driver
|
||||
|
||||
#include <linux/bits.h>
|
||||
#include <linux/clk.h>
|
||||
#include <linux/device.h>
|
||||
#include <linux/io.h>
|
||||
|
|
@ -47,18 +48,36 @@ static const struct reg_default tegra264_adx_reg_defaults[] = {
|
|||
|
||||
static void tegra210_adx_write_map_ram(struct tegra210_adx *adx)
|
||||
{
|
||||
const unsigned int bits_per_mask = BITS_PER_TYPE(*adx->byte_mask);
|
||||
int i;
|
||||
|
||||
memset(adx->byte_mask, 0,
|
||||
adx->soc_data->byte_mask_size * sizeof(*adx->byte_mask));
|
||||
|
||||
regmap_write(adx->regmap, TEGRA210_ADX_CFG_RAM_CTRL +
|
||||
adx->soc_data->cya_offset,
|
||||
TEGRA210_ADX_CFG_RAM_CTRL_SEQ_ACCESS_EN |
|
||||
TEGRA210_ADX_CFG_RAM_CTRL_ADDR_INIT_EN |
|
||||
TEGRA210_ADX_CFG_RAM_CTRL_RW_WRITE);
|
||||
|
||||
for (i = 0; i < adx->soc_data->ram_depth; i++)
|
||||
for (i = 0; i < adx->soc_data->ram_depth; i++) {
|
||||
u32 word = 0;
|
||||
int b;
|
||||
|
||||
for (b = 0; b < TEGRA_ADX_SLOTS_PER_WORD; b++) {
|
||||
unsigned int slot = i * TEGRA_ADX_SLOTS_PER_WORD + b;
|
||||
u16 val = adx->map[slot];
|
||||
|
||||
if (val >= 256)
|
||||
continue;
|
||||
|
||||
word |= (u32)val << (b * BITS_PER_BYTE);
|
||||
adx->byte_mask[slot / bits_per_mask] |=
|
||||
1U << (slot % bits_per_mask);
|
||||
}
|
||||
regmap_write(adx->regmap, TEGRA210_ADX_CFG_RAM_DATA +
|
||||
adx->soc_data->cya_offset,
|
||||
adx->map[i]);
|
||||
adx->soc_data->cya_offset, word);
|
||||
}
|
||||
|
||||
for (i = 0; i < adx->soc_data->byte_mask_size; i++)
|
||||
regmap_write(adx->regmap,
|
||||
|
|
@ -192,27 +211,10 @@ static int tegra210_adx_get_byte_map(struct snd_kcontrol *kcontrol,
|
|||
{
|
||||
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
|
||||
struct tegra210_adx *adx = snd_soc_component_get_drvdata(cmpnt);
|
||||
struct soc_mixer_control *mc;
|
||||
unsigned char *bytes_map = (unsigned char *)adx->map;
|
||||
int enabled;
|
||||
struct soc_mixer_control *mc =
|
||||
(struct soc_mixer_control *)kcontrol->private_value;
|
||||
|
||||
mc = (struct soc_mixer_control *)kcontrol->private_value;
|
||||
enabled = adx->byte_mask[mc->reg / 32] & (1 << (mc->reg % 32));
|
||||
|
||||
/*
|
||||
* TODO: Simplify this logic to just return from bytes_map[]
|
||||
*
|
||||
* Presently below is required since bytes_map[] is
|
||||
* tightly packed and cannot store the control value of 256.
|
||||
* Byte mask state is used to know if 256 needs to be returned.
|
||||
* Note that for control value of 256, the put() call stores 0
|
||||
* in the bytes_map[] and disables the corresponding bit in
|
||||
* byte_mask[].
|
||||
*/
|
||||
if (enabled)
|
||||
ucontrol->value.integer.value[0] = bytes_map[mc->reg];
|
||||
else
|
||||
ucontrol->value.integer.value[0] = 256;
|
||||
ucontrol->value.integer.value[0] = adx->map[mc->reg];
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
|
@ -222,23 +224,22 @@ static int tegra210_adx_put_byte_map(struct snd_kcontrol *kcontrol,
|
|||
{
|
||||
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
|
||||
struct tegra210_adx *adx = snd_soc_component_get_drvdata(cmpnt);
|
||||
unsigned char *bytes_map = (unsigned char *)adx->map;
|
||||
int value = ucontrol->value.integer.value[0];
|
||||
struct soc_mixer_control *mc =
|
||||
(struct soc_mixer_control *)kcontrol->private_value;
|
||||
unsigned int mask_val = adx->byte_mask[mc->reg / 32];
|
||||
unsigned int value = ucontrol->value.integer.value[0];
|
||||
|
||||
if (value >= 0 && value <= 255)
|
||||
mask_val |= (1 << (mc->reg % 32));
|
||||
else
|
||||
mask_val &= ~(1 << (mc->reg % 32));
|
||||
/*
|
||||
* Match the previous behaviour: any value outside [0, 255] is
|
||||
* treated as the "disabled" sentinel (256). Negative values from
|
||||
* userspace fold in through the unsigned cast and are caught here.
|
||||
*/
|
||||
if (value > 255)
|
||||
value = 256;
|
||||
|
||||
if (mask_val == adx->byte_mask[mc->reg / 32])
|
||||
if (adx->map[mc->reg] == value)
|
||||
return 0;
|
||||
|
||||
/* Update byte map and slot */
|
||||
bytes_map[mc->reg] = value % 256;
|
||||
adx->byte_mask[mc->reg / 32] = mask_val;
|
||||
adx->map[mc->reg] = value;
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
|
@ -679,7 +680,7 @@ static int tegra210_adx_platform_probe(struct platform_device *pdev)
|
|||
const struct of_device_id *match;
|
||||
struct tegra210_adx_soc_data *soc_data;
|
||||
void __iomem *regs;
|
||||
int err;
|
||||
int err, i;
|
||||
|
||||
adx = devm_kzalloc(dev, sizeof(*adx), GFP_KERNEL);
|
||||
if (!adx)
|
||||
|
|
@ -703,17 +704,21 @@ static int tegra210_adx_platform_probe(struct platform_device *pdev)
|
|||
|
||||
regcache_cache_only(adx->regmap, true);
|
||||
|
||||
adx->map = devm_kzalloc(dev, soc_data->ram_depth * sizeof(*adx->map),
|
||||
GFP_KERNEL);
|
||||
adx->map = devm_kcalloc(dev,
|
||||
soc_data->ram_depth * TEGRA_ADX_SLOTS_PER_WORD,
|
||||
sizeof(*adx->map), GFP_KERNEL);
|
||||
if (!adx->map)
|
||||
return -ENOMEM;
|
||||
|
||||
adx->byte_mask = devm_kzalloc(dev,
|
||||
soc_data->byte_mask_size * sizeof(*adx->byte_mask),
|
||||
GFP_KERNEL);
|
||||
adx->byte_mask = devm_kcalloc(dev, soc_data->byte_mask_size,
|
||||
sizeof(*adx->byte_mask), GFP_KERNEL);
|
||||
if (!adx->byte_mask)
|
||||
return -ENOMEM;
|
||||
|
||||
/* Initialise all byte map slots as disabled (value 256). */
|
||||
for (i = 0; i < soc_data->ram_depth * TEGRA_ADX_SLOTS_PER_WORD; i++)
|
||||
adx->map[i] = 256;
|
||||
|
||||
tegra210_adx_dais[TEGRA_ADX_IN_DAI_ID].playback.channels_max =
|
||||
adx->soc_data->max_ch;
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@
|
|||
#ifndef __TEGRA210_ADX_H__
|
||||
#define __TEGRA210_ADX_H__
|
||||
|
||||
#include <linux/types.h>
|
||||
|
||||
/* Register offsets from TEGRA210_ADX*_BASE */
|
||||
#define TEGRA210_ADX_RX_STATUS 0x0c
|
||||
#define TEGRA210_ADX_RX_INT_STATUS 0x10
|
||||
|
|
@ -61,6 +63,7 @@
|
|||
#define TEGRA210_ADX_SOFT_RESET_SOFT_DEFAULT (0 << TEGRA210_ADX_SOFT_RESET_SOFT_RESET_SHIFT)
|
||||
|
||||
#define TEGRA210_ADX_AUDIOCIF_CH_STRIDE 4
|
||||
#define TEGRA_ADX_SLOTS_PER_WORD 4
|
||||
#define TEGRA210_ADX_RAM_DEPTH 16
|
||||
#define TEGRA210_ADX_MAP_STREAM_NUMBER_SHIFT 6
|
||||
#define TEGRA210_ADX_MAP_WORD_NUMBER_SHIFT 2
|
||||
|
|
@ -88,8 +91,8 @@ struct tegra210_adx_soc_data {
|
|||
|
||||
struct tegra210_adx {
|
||||
struct regmap *regmap;
|
||||
unsigned int *map;
|
||||
unsigned int *byte_mask;
|
||||
u16 *map;
|
||||
const struct tegra210_adx_soc_data *soc_data;
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
//
|
||||
// tegra210_amx.c - Tegra210 AMX driver
|
||||
|
||||
#include <linux/bits.h>
|
||||
#include <linux/clk.h>
|
||||
#include <linux/device.h>
|
||||
#include <linux/io.h>
|
||||
|
|
@ -60,16 +61,35 @@ static const struct reg_default tegra264_amx_reg_defaults[] = {
|
|||
|
||||
static void tegra210_amx_write_map_ram(struct tegra210_amx *amx)
|
||||
{
|
||||
const unsigned int bits_per_mask = BITS_PER_TYPE(*amx->byte_mask);
|
||||
int i;
|
||||
|
||||
memset(amx->byte_mask, 0,
|
||||
amx->soc_data->byte_mask_size * sizeof(*amx->byte_mask));
|
||||
|
||||
regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_CTRL + amx->soc_data->reg_offset,
|
||||
TEGRA210_AMX_CFG_RAM_CTRL_SEQ_ACCESS_EN |
|
||||
TEGRA210_AMX_CFG_RAM_CTRL_ADDR_INIT_EN |
|
||||
TEGRA210_AMX_CFG_RAM_CTRL_RW_WRITE);
|
||||
|
||||
for (i = 0; i < amx->soc_data->ram_depth; i++)
|
||||
for (i = 0; i < amx->soc_data->ram_depth; i++) {
|
||||
u32 word = 0;
|
||||
int b;
|
||||
|
||||
for (b = 0; b < TEGRA_AMX_SLOTS_PER_WORD; b++) {
|
||||
unsigned int slot = i * TEGRA_AMX_SLOTS_PER_WORD + b;
|
||||
u16 val = amx->map[slot];
|
||||
|
||||
if (val >= 256)
|
||||
continue;
|
||||
|
||||
word |= (u32)val << (b * BITS_PER_BYTE);
|
||||
amx->byte_mask[slot / bits_per_mask] |=
|
||||
1U << (slot % bits_per_mask);
|
||||
}
|
||||
regmap_write(amx->regmap, TEGRA210_AMX_CFG_RAM_DATA + amx->soc_data->reg_offset,
|
||||
amx->map[i]);
|
||||
word);
|
||||
}
|
||||
|
||||
for (i = 0; i < amx->soc_data->byte_mask_size; i++)
|
||||
regmap_write(amx->regmap,
|
||||
|
|
@ -214,26 +234,8 @@ static int tegra210_amx_get_byte_map(struct snd_kcontrol *kcontrol,
|
|||
struct soc_mixer_control *mc =
|
||||
(struct soc_mixer_control *)kcontrol->private_value;
|
||||
struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
|
||||
unsigned char *bytes_map = (unsigned char *)amx->map;
|
||||
int reg = mc->reg;
|
||||
int enabled;
|
||||
|
||||
enabled = amx->byte_mask[reg / 32] & (1 << (reg % 32));
|
||||
|
||||
/*
|
||||
* TODO: Simplify this logic to just return from bytes_map[]
|
||||
*
|
||||
* Presently below is required since bytes_map[] is
|
||||
* tightly packed and cannot store the control value of 256.
|
||||
* Byte mask state is used to know if 256 needs to be returned.
|
||||
* Note that for control value of 256, the put() call stores 0
|
||||
* in the bytes_map[] and disables the corresponding bit in
|
||||
* byte_mask[].
|
||||
*/
|
||||
if (enabled)
|
||||
ucontrol->value.integer.value[0] = bytes_map[reg];
|
||||
else
|
||||
ucontrol->value.integer.value[0] = 256;
|
||||
ucontrol->value.integer.value[0] = amx->map[mc->reg];
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
|
@ -245,22 +247,20 @@ static int tegra210_amx_put_byte_map(struct snd_kcontrol *kcontrol,
|
|||
(struct soc_mixer_control *)kcontrol->private_value;
|
||||
struct snd_soc_component *cmpnt = snd_kcontrol_chip(kcontrol);
|
||||
struct tegra210_amx *amx = snd_soc_component_get_drvdata(cmpnt);
|
||||
unsigned char *bytes_map = (unsigned char *)amx->map;
|
||||
int reg = mc->reg;
|
||||
int value = ucontrol->value.integer.value[0];
|
||||
unsigned int mask_val = amx->byte_mask[reg / 32];
|
||||
unsigned int value = ucontrol->value.integer.value[0];
|
||||
|
||||
if (value >= 0 && value <= 255)
|
||||
mask_val |= (1 << (reg % 32));
|
||||
else
|
||||
mask_val &= ~(1 << (reg % 32));
|
||||
/*
|
||||
* Match the previous behaviour: any value outside [0, 255] is
|
||||
* treated as the "disabled" sentinel (256). Negative values from
|
||||
* userspace fold in through the unsigned cast and are caught here.
|
||||
*/
|
||||
if (value > 255)
|
||||
value = 256;
|
||||
|
||||
if (mask_val == amx->byte_mask[reg / 32])
|
||||
if (amx->map[mc->reg] == value)
|
||||
return 0;
|
||||
|
||||
/* Update byte map and slot */
|
||||
bytes_map[reg] = value % 256;
|
||||
amx->byte_mask[reg / 32] = mask_val;
|
||||
amx->map[mc->reg] = value;
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
|
@ -729,7 +729,7 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
|
|||
struct device *dev = &pdev->dev;
|
||||
struct tegra210_amx *amx;
|
||||
void __iomem *regs;
|
||||
int err;
|
||||
int err, i;
|
||||
|
||||
amx = devm_kzalloc(dev, sizeof(*amx), GFP_KERNEL);
|
||||
if (!amx)
|
||||
|
|
@ -751,17 +751,21 @@ static int tegra210_amx_platform_probe(struct platform_device *pdev)
|
|||
|
||||
regcache_cache_only(amx->regmap, true);
|
||||
|
||||
amx->map = devm_kzalloc(dev, amx->soc_data->ram_depth * sizeof(*amx->map),
|
||||
GFP_KERNEL);
|
||||
amx->map = devm_kcalloc(dev,
|
||||
amx->soc_data->ram_depth * TEGRA_AMX_SLOTS_PER_WORD,
|
||||
sizeof(*amx->map), GFP_KERNEL);
|
||||
if (!amx->map)
|
||||
return -ENOMEM;
|
||||
|
||||
amx->byte_mask = devm_kzalloc(dev,
|
||||
amx->soc_data->byte_mask_size * sizeof(*amx->byte_mask),
|
||||
GFP_KERNEL);
|
||||
amx->byte_mask = devm_kcalloc(dev, amx->soc_data->byte_mask_size,
|
||||
sizeof(*amx->byte_mask), GFP_KERNEL);
|
||||
if (!amx->byte_mask)
|
||||
return -ENOMEM;
|
||||
|
||||
/* Initialise all byte map slots as disabled (value 256). */
|
||||
for (i = 0; i < amx->soc_data->ram_depth * TEGRA_AMX_SLOTS_PER_WORD; i++)
|
||||
amx->map[i] = 256;
|
||||
|
||||
tegra210_amx_dais[TEGRA_AMX_OUT_DAI_ID].capture.channels_max =
|
||||
amx->soc_data->max_ch;
|
||||
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@
|
|||
#ifndef __TEGRA210_AMX_H__
|
||||
#define __TEGRA210_AMX_H__
|
||||
|
||||
#include <linux/types.h>
|
||||
|
||||
/* Register offsets from TEGRA210_AMX*_BASE */
|
||||
#define TEGRA210_AMX_RX_STATUS 0x0c
|
||||
#define TEGRA210_AMX_RX_INT_STATUS 0x10
|
||||
|
|
@ -73,6 +75,7 @@
|
|||
#define TEGRA210_AMX_SOFT_RESET_SOFT_RESET_MASK TEGRA210_AMX_SOFT_RESET_SOFT_EN
|
||||
|
||||
#define TEGRA210_AMX_AUDIOCIF_CH_STRIDE 4
|
||||
#define TEGRA_AMX_SLOTS_PER_WORD 4
|
||||
#define TEGRA210_AMX_RAM_DEPTH 16
|
||||
#define TEGRA210_AMX_MAP_STREAM_NUM_SHIFT 6
|
||||
#define TEGRA210_AMX_MAP_WORD_NUM_SHIFT 2
|
||||
|
|
@ -105,8 +108,8 @@ struct tegra210_amx_soc_data {
|
|||
|
||||
struct tegra210_amx {
|
||||
const struct tegra210_amx_soc_data *soc_data;
|
||||
unsigned int *map;
|
||||
unsigned int *byte_mask;
|
||||
u16 *map;
|
||||
struct regmap *regmap;
|
||||
};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user