Currently there are 3 locks being used when accessing the chip, one
in the driver and one in each regmap. Reduce that to one driver only
lock that protects all regmap and regcache accesses.
Signed-off-by: Patrick Rudolph <[email protected]>
---
drivers/pinctrl/pinctrl-cy8c95x0.c | 32 ++++++++++++++++--------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 981c569bd671..ca54d91fdc77 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -453,7 +453,6 @@ cy8c95x0_mux_reg_read(void *context, unsigned int off, unsigned int *val)
u8 port = CY8C95X0_MUX_REGMAP_TO_PORT(off);
int ret, reg = CY8C95X0_MUX_REGMAP_TO_REG(off);
- mutex_lock(&chip->i2c_lock);
/* Select the correct bank */
ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
if (ret < 0)
@@ -463,11 +462,7 @@ cy8c95x0_mux_reg_read(void *context, unsigned int off, unsigned int *val)
* Read the register through direct access regmap. The target range
* is marked volatile.
*/
- ret = regmap_read(chip->regmap, reg, val);
-out:
- mutex_unlock(&chip->i2c_lock);
-
- return ret;
+ return regmap_read(chip->regmap, reg, val);
}
static int
@@ -477,7 +472,6 @@ cy8c95x0_mux_reg_write(void *context, unsigned int off, unsigned int val)
u8 port = CY8C95X0_MUX_REGMAP_TO_PORT(off);
int ret, reg = CY8C95X0_MUX_REGMAP_TO_REG(off);
- mutex_lock(&chip->i2c_lock);
/* Select the correct bank */
ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
if (ret < 0)
@@ -487,11 +481,7 @@ cy8c95x0_mux_reg_write(void *context, unsigned int off, unsigned int val)
* Write the register through direct access regmap. The target range
* is marked volatile.
*/
- ret = regmap_write(chip->regmap, reg, val);
-out:
- mutex_unlock(&chip->i2c_lock);
-
- return ret;
+ return regmap_write(chip->regmap, reg, val);
}
static bool cy8c95x0_mux_accessible_register(struct device *dev, unsigned int off)
@@ -524,6 +514,7 @@ static const struct regmap_config cy8c95x0_muxed_regmap = {
.num_reg_defaults_raw = MUXED_STRIDE * BANK_SZ,
.readable_reg = cy8c95x0_mux_accessible_register,
.writeable_reg = cy8c95x0_mux_accessible_register,
+ .disable_locking = true,
};
/* Direct access regmap */
@@ -542,6 +533,7 @@ static const struct regmap_config cy8c95x0_i2c_regmap = {
.cache_type = REGCACHE_FLAT,
.max_register = CY8C95X0_COMMAND,
+ .disable_locking = true,
};
static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip,
@@ -559,6 +551,8 @@ static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip
if (reg == CY8C95X0_PORTSEL)
return -EINVAL;
+ mutex_lock(&chip->i2c_lock);
+
/* Registers behind the PORTSEL mux have their own regmap */
if (cy8c95x0_muxed_register(reg)) {
regmap = chip->muxed_regmap;
@@ -574,7 +568,7 @@ static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip
ret = regmap_update_bits_base(regmap, off, mask, val, change, async, force);
if (ret < 0)
- return ret;
+ goto out;
/* Update the cache when a WC bit is written */
if (cy8c95x0_wc_register(reg) && (mask & val)) {
@@ -595,6 +589,8 @@ static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip
regcache_cache_only(regmap, false);
}
}
+out:
+ mutex_unlock(&chip->i2c_lock);
return ret;
}
@@ -667,7 +663,9 @@ static int cy8c95x0_regmap_read(struct cy8c95x0_pinctrl *chip, unsigned int reg,
unsigned int port, unsigned int *read_val)
{
struct regmap *regmap;
- int off;
+ int off, ret;
+
+ mutex_lock(&chip->i2c_lock);
/* Registers behind the PORTSEL mux have their own regmap */
if (cy8c95x0_muxed_register(reg)) {
@@ -682,7 +680,11 @@ static int cy8c95x0_regmap_read(struct cy8c95x0_pinctrl *chip, unsigned int reg,
off = reg;
}
- return regmap_read(regmap, off, read_val);
+ ret = regmap_read(regmap, off, read_val);
+
+ mutex_unlock(&chip->i2c_lock);
+
+ return ret;
}
static int cy8c95x0_write_regs_mask(struct cy8c95x0_pinctrl *chip, int reg,
--
2.44.0
Instead of implementing a custom register paging mechanism in
the driver use the existing regmap ranges feature.
Signed-off-by: Patrick Rudolph <[email protected]>
---
drivers/pinctrl/pinctrl-cy8c95x0.c | 179 +++++++++--------------------
1 file changed, 53 insertions(+), 126 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index ca54d91fdc77..9570de598193 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -58,9 +58,14 @@
#define CY8C95X0_PIN_TO_OFFSET(x) (((x) >= 20) ? ((x) + 4) : (x))
-#define CY8C95X0_MUX_REGMAP_TO_PORT(x) ((x) / MUXED_STRIDE)
-#define CY8C95X0_MUX_REGMAP_TO_REG(x) (((x) % MUXED_STRIDE) + CY8C95X0_INTMASK)
-#define CY8C95X0_MUX_REGMAP_TO_OFFSET(x, p) ((x) - CY8C95X0_INTMASK + (p) * MUXED_STRIDE)
+#define MAX_BANK 8
+#define BANK_SZ 8
+#define MAX_LINE (MAX_BANK * BANK_SZ)
+#define MUXED_STRIDE (CY8C95X0_DRV_HIZ - CY8C95X0_INTMASK)
+#define CY8C95X0_GPIO_MASK GENMASK(7, 0)
+#define CY8C95X0_VIRTUAL (CY8C95X0_COMMAND + 1)
+#define CY8C95X0_MUX_REGMAP_TO_OFFSET(x, p) \
+ (CY8C95X0_VIRTUAL + (x) - CY8C95X0_INTMASK + (p) * MUXED_STRIDE)
static const struct i2c_device_id cy8c95x0_id[] = {
{ "cy8c9520", 20, },
@@ -120,18 +125,11 @@ static const struct dmi_system_id cy8c95x0_dmi_acpi_irq_info[] = {
{}
};
-#define MAX_BANK 8
-#define BANK_SZ 8
-#define MAX_LINE (MAX_BANK * BANK_SZ)
-#define MUXED_STRIDE 16
-#define CY8C95X0_GPIO_MASK GENMASK(7, 0)
-
/**
* struct cy8c95x0_pinctrl - driver data
* @regmap: Device's regmap. Only direct access registers.
- * @muxed_regmap: Regmap for all muxed registers.
* @irq_lock: IRQ bus lock
- * @i2c_lock: Mutex for the device internal mux register
+ * @i2c_lock: Mutex to hold while using the regmap
* @irq_mask: I/O bits affected by interrupts
* @irq_trig_raise: I/O bits affected by raising voltage level
* @irq_trig_fall: I/O bits affected by falling voltage level
@@ -152,7 +150,6 @@ static const struct dmi_system_id cy8c95x0_dmi_acpi_irq_info[] = {
*/
struct cy8c95x0_pinctrl {
struct regmap *regmap;
- struct regmap *muxed_regmap;
struct mutex irq_lock;
struct mutex i2c_lock;
DECLARE_BITMAP(irq_mask, MAX_LINE);
@@ -331,6 +328,9 @@ static int cypress_get_pin_mask(struct cy8c95x0_pinctrl *chip, unsigned int pin)
static bool cy8c95x0_readable_register(struct device *dev, unsigned int reg)
{
+ if (reg >= CY8C95X0_VIRTUAL)
+ return true;
+
switch (reg) {
case 0x24 ... 0x27:
return false;
@@ -341,6 +341,9 @@ static bool cy8c95x0_readable_register(struct device *dev, unsigned int reg)
static bool cy8c95x0_writeable_register(struct device *dev, unsigned int reg)
{
+ if (reg >= CY8C95X0_VIRTUAL)
+ return true;
+
switch (reg) {
case CY8C95X0_INPUT_(0) ... CY8C95X0_INPUT_(7):
return false;
@@ -433,106 +436,33 @@ static bool cy8c95x0_quick_path_register(unsigned int reg)
}
}
-static const struct reg_default cy8c95x0_reg_defaults[] = {
- { CY8C95X0_OUTPUT_(0), GENMASK(7, 0) },
- { CY8C95X0_OUTPUT_(1), GENMASK(7, 0) },
- { CY8C95X0_OUTPUT_(2), GENMASK(7, 0) },
- { CY8C95X0_OUTPUT_(3), GENMASK(7, 0) },
- { CY8C95X0_OUTPUT_(4), GENMASK(7, 0) },
- { CY8C95X0_OUTPUT_(5), GENMASK(7, 0) },
- { CY8C95X0_OUTPUT_(6), GENMASK(7, 0) },
- { CY8C95X0_OUTPUT_(7), GENMASK(7, 0) },
- { CY8C95X0_PORTSEL, 0 },
- { CY8C95X0_PWMSEL, 0 },
-};
-
-static int
-cy8c95x0_mux_reg_read(void *context, unsigned int off, unsigned int *val)
-{
- struct cy8c95x0_pinctrl *chip = context;
- u8 port = CY8C95X0_MUX_REGMAP_TO_PORT(off);
- int ret, reg = CY8C95X0_MUX_REGMAP_TO_REG(off);
-
- /* Select the correct bank */
- ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
- if (ret < 0)
- goto out;
-
- /*
- * Read the register through direct access regmap. The target range
- * is marked volatile.
- */
- return regmap_read(chip->regmap, reg, val);
-}
-
-static int
-cy8c95x0_mux_reg_write(void *context, unsigned int off, unsigned int val)
-{
- struct cy8c95x0_pinctrl *chip = context;
- u8 port = CY8C95X0_MUX_REGMAP_TO_PORT(off);
- int ret, reg = CY8C95X0_MUX_REGMAP_TO_REG(off);
-
- /* Select the correct bank */
- ret = regmap_write(chip->regmap, CY8C95X0_PORTSEL, port);
- if (ret < 0)
- goto out;
-
- /*
- * Write the register through direct access regmap. The target range
- * is marked volatile.
- */
- return regmap_write(chip->regmap, reg, val);
-}
-
-static bool cy8c95x0_mux_accessible_register(struct device *dev, unsigned int off)
-{
- struct i2c_client *i2c = to_i2c_client(dev);
- struct cy8c95x0_pinctrl *chip = i2c_get_clientdata(i2c);
- u8 port = CY8C95X0_MUX_REGMAP_TO_PORT(off);
- u8 reg = CY8C95X0_MUX_REGMAP_TO_REG(off);
-
- if (port >= chip->nport)
- return false;
-
- return cy8c95x0_muxed_register(reg);
-}
-
-static struct regmap_bus cy8c95x0_regmap_bus = {
- .reg_read = cy8c95x0_mux_reg_read,
- .reg_write = cy8c95x0_mux_reg_write,
-};
-
-/* Regmap for muxed registers CY8C95X0_INTMASK - CY8C95X0_DRV_HIZ */
-static const struct regmap_config cy8c95x0_muxed_regmap = {
- .name = "muxed",
- .reg_bits = 8,
- .val_bits = 8,
- .cache_type = REGCACHE_FLAT,
- .use_single_read = true,
- .use_single_write = true,
- .max_register = MUXED_STRIDE * BANK_SZ,
- .num_reg_defaults_raw = MUXED_STRIDE * BANK_SZ,
- .readable_reg = cy8c95x0_mux_accessible_register,
- .writeable_reg = cy8c95x0_mux_accessible_register,
- .disable_locking = true,
+static const struct regmap_range_cfg cy8c95x0_ranges[] = {
+ {
+ .range_min = CY8C95X0_VIRTUAL,
+ .range_max = 0, /* Updated at runtime */
+ .selector_reg = CY8C95X0_PORTSEL,
+ .selector_mask = 0x07,
+ .selector_shift = 0x0,
+ .window_start = CY8C95X0_INTMASK,
+ .window_len = MUXED_STRIDE,
+ }
};
-/* Direct access regmap */
-static const struct regmap_config cy8c95x0_i2c_regmap = {
- .name = "direct",
+static const struct regmap_config cy8c9520_i2c_regmap = {
.reg_bits = 8,
.val_bits = 8,
- .reg_defaults = cy8c95x0_reg_defaults,
- .num_reg_defaults = ARRAY_SIZE(cy8c95x0_reg_defaults),
-
.readable_reg = cy8c95x0_readable_register,
.writeable_reg = cy8c95x0_writeable_register,
.volatile_reg = cy8c95x0_volatile_register,
.precious_reg = cy8c95x0_precious_register,
.cache_type = REGCACHE_FLAT,
- .max_register = CY8C95X0_COMMAND,
+ .ranges = NULL, /* Updated at runtime */
+ .num_ranges = 1,
+ .max_register = 0, /* Updated at runtime */
+ .num_reg_defaults_raw = 0, /* Updated at runtime */
+ .use_single_read = true, /* Workaround for regcache bug */
.disable_locking = true,
};
@@ -544,7 +474,6 @@ static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip
bool *change, bool async,
bool force)
{
- struct regmap *regmap;
int ret, off, i, read_val;
/* Caller should never modify PORTSEL directly */
@@ -553,12 +482,10 @@ static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip
mutex_lock(&chip->i2c_lock);
- /* Registers behind the PORTSEL mux have their own regmap */
+ /* Registers behind the PORTSEL mux have their own range in regmap */
if (cy8c95x0_muxed_register(reg)) {
- regmap = chip->muxed_regmap;
off = CY8C95X0_MUX_REGMAP_TO_OFFSET(reg, port);
} else {
- regmap = chip->regmap;
/* Quick path direct access registers honor the port argument */
if (cy8c95x0_quick_path_register(reg))
off = reg + port;
@@ -566,7 +493,7 @@ static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip
off = reg;
}
- ret = regmap_update_bits_base(regmap, off, mask, val, change, async, force);
+ ret = regmap_update_bits_base(chip->regmap, off, mask, val, change, async, force);
if (ret < 0)
goto out;
@@ -577,16 +504,16 @@ static inline int cy8c95x0_regmap_update_bits_base(struct cy8c95x0_pinctrl *chip
continue;
off = CY8C95X0_MUX_REGMAP_TO_OFFSET(i, port);
- ret = regmap_read(regmap, off, &read_val);
+ ret = regmap_read(chip->regmap, off, &read_val);
if (ret < 0)
continue;
if (!(read_val & mask & val))
continue;
- regcache_cache_only(regmap, true);
- regmap_update_bits(regmap, off, mask & val, 0);
- regcache_cache_only(regmap, false);
+ regcache_cache_only(chip->regmap, true);
+ regmap_update_bits(chip->regmap, off, mask & val, 0);
+ regcache_cache_only(chip->regmap, false);
}
}
out:
@@ -662,17 +589,14 @@ static int cy8c95x0_regmap_update_bits(struct cy8c95x0_pinctrl *chip, unsigned i
static int cy8c95x0_regmap_read(struct cy8c95x0_pinctrl *chip, unsigned int reg,
unsigned int port, unsigned int *read_val)
{
- struct regmap *regmap;
int off, ret;
mutex_lock(&chip->i2c_lock);
- /* Registers behind the PORTSEL mux have their own regmap */
+ /* Registers behind the PORTSEL mux have their own range in regmap */
if (cy8c95x0_muxed_register(reg)) {
- regmap = chip->muxed_regmap;
off = CY8C95X0_MUX_REGMAP_TO_OFFSET(reg, port);
} else {
- regmap = chip->regmap;
/* Quick path direct access registers honor the port argument */
if (cy8c95x0_quick_path_register(reg))
off = reg + port;
@@ -680,7 +604,7 @@ static int cy8c95x0_regmap_read(struct cy8c95x0_pinctrl *chip, unsigned int reg,
off = reg;
}
- ret = regmap_read(regmap, off, read_val);
+ ret = regmap_read(chip->regmap, off, read_val);
mutex_unlock(&chip->i2c_lock);
@@ -1513,6 +1437,8 @@ static int cy8c95x0_detect(struct i2c_client *client,
static int cy8c95x0_probe(struct i2c_client *client)
{
struct cy8c95x0_pinctrl *chip;
+ struct regmap_config regmap_conf;
+ struct regmap_range_cfg regmap_range_conf;
struct regulator *reg;
int ret;
@@ -1532,15 +1458,20 @@ static int cy8c95x0_probe(struct i2c_client *client)
chip->tpin = chip->driver_data & CY8C95X0_GPIO_MASK;
chip->nport = DIV_ROUND_UP(CY8C95X0_PIN_TO_OFFSET(chip->tpin), BANK_SZ);
+ memcpy(®map_range_conf, &cy8c95x0_ranges[0], sizeof(regmap_range_conf));
+
switch (chip->tpin) {
case 20:
strscpy(chip->name, cy8c95x0_id[0].name, I2C_NAME_SIZE);
+ regmap_range_conf.range_max = CY8C95X0_VIRTUAL + 3 * MUXED_STRIDE;
break;
case 40:
strscpy(chip->name, cy8c95x0_id[1].name, I2C_NAME_SIZE);
+ regmap_range_conf.range_max = CY8C95X0_VIRTUAL + 6 * MUXED_STRIDE;
break;
case 60:
strscpy(chip->name, cy8c95x0_id[2].name, I2C_NAME_SIZE);
+ regmap_range_conf.range_max = CY8C95X0_VIRTUAL + 8 * MUXED_STRIDE;
break;
default:
return -ENODEV;
@@ -1573,22 +1504,18 @@ static int cy8c95x0_probe(struct i2c_client *client)
gpiod_set_consumer_name(chip->gpio_reset, "CY8C95X0 RESET");
}
- /* Generic regmap for direct access registers */
- chip->regmap = devm_regmap_init_i2c(client, &cy8c95x0_i2c_regmap);
+ /* Regmap for direct and paged registers */
+ memcpy(®map_conf, &cy8c9520_i2c_regmap, sizeof(regmap_conf));
+ regmap_conf.ranges = ®map_range_conf;
+ regmap_conf.max_register = regmap_range_conf.range_max;
+ regmap_conf.num_reg_defaults_raw = regmap_range_conf.range_max;
+
+ chip->regmap = devm_regmap_init_i2c(client, ®map_conf);
if (IS_ERR(chip->regmap)) {
ret = PTR_ERR(chip->regmap);
goto err_exit;
}
- /* Port specific regmap behind PORTSEL mux */
- chip->muxed_regmap = devm_regmap_init(&client->dev, &cy8c95x0_regmap_bus,
- chip, &cy8c95x0_muxed_regmap);
- if (IS_ERR(chip->muxed_regmap)) {
- ret = dev_err_probe(&client->dev, PTR_ERR(chip->muxed_regmap),
- "Failed to register muxed regmap\n");
- goto err_exit;
- }
-
bitmap_zero(chip->push_pull, MAX_LINE);
bitmap_zero(chip->shiftmask, MAX_LINE);
bitmap_set(chip->shiftmask, 0, 20);
--
2.44.0
Use REGCACHE_MAPLE instead of REGCACHE_FLAT.
Signed-off-by: Patrick Rudolph <[email protected]>
---
drivers/pinctrl/pinctrl-cy8c95x0.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-cy8c95x0.c b/drivers/pinctrl/pinctrl-cy8c95x0.c
index 9570de598193..4efb8b5cc2d3 100644
--- a/drivers/pinctrl/pinctrl-cy8c95x0.c
+++ b/drivers/pinctrl/pinctrl-cy8c95x0.c
@@ -457,7 +457,7 @@ static const struct regmap_config cy8c9520_i2c_regmap = {
.volatile_reg = cy8c95x0_volatile_register,
.precious_reg = cy8c95x0_precious_register,
- .cache_type = REGCACHE_FLAT,
+ .cache_type = REGCACHE_MAPLE,
.ranges = NULL, /* Updated at runtime */
.num_ranges = 1,
.max_register = 0, /* Updated at runtime */
--
2.44.0
On Tue, May 21, 2024 at 6:26 PM Patrick Rudolph
<[email protected]> wrote:
>
> Currently there are 3 locks being used when accessing the chip, one
> in the driver and one in each regmap. Reduce that to one driver only
> lock that protects all regmap and regcache accesses.
Right. But please consider converting the driver to use cleanup.h.
Dunno if it requires a separate patch or can be folded into this one
as it seems you anyway touch almost all mutex calls in the code.
Linus?
--
With Best Regards,
Andy Shevchenko
On Tue, May 21, 2024 at 6:26 PM Patrick Rudolph
<[email protected]> wrote:
>
> Instead of implementing a custom register paging mechanism in
> the driver use the existing regmap ranges feature.
driver, use
..
> +static const struct regmap_range_cfg cy8c95x0_ranges[] = {
> + {
> + .range_min = CY8C95X0_VIRTUAL,
> + .range_max = 0, /* Updated at runtime */
This and similar comments are misleading since the data is defined as
const. Updated --> Filled or alike here and elsewhere.
> + .selector_reg = CY8C95X0_PORTSEL,
> + .selector_mask = 0x07,
> + .selector_shift = 0x0,
> + .window_start = CY8C95X0_INTMASK,
> + .window_len = MUXED_STRIDE,
> + }
> };
..
> + regcache_cache_only(chip->regmap, true);
> + regmap_update_bits(chip->regmap, off, mask & val, 0);
> + regcache_cache_only(chip->regmap, false);
A side note: It's strange to see mask & val, 0 in the parameters of
regmap calls. Perhaps refactor this (in a separate patch) to use
standard approach?
..
> + memcpy(®map_range_conf, &cy8c95x0_ranges[0], sizeof(regmap_range_conf));
memcpy(®map_range_conf, cy8c95x0_ranges, sizeof(regmap_range_conf));
..
Note, if you are not using --histogram diff algo, please start using
it from the next version of this series.
P.S. I will test the next version on Intel Galileo Gen1 as currently I
have some issues with the HW I need to fix first.
--
With Best Regards,
Andy Shevchenko
On Tue, May 21, 2024 at 6:26 PM Patrick Rudolph
<[email protected]> wrote:
>
> Use REGCACHE_MAPLE instead of REGCACHE_FLAT.
Reviewed-by: Andy Shevchenko <[email protected]>
--
With Best Regards,
Andy Shevchenko
On Tue, May 21, 2024 at 7:25 PM Andy Shevchenko
<[email protected]> wrote:
> On Tue, May 21, 2024 at 6:26 PM Patrick Rudolph
> <[email protected]> wrote:
> >
> > Currently there are 3 locks being used when accessing the chip, one
> > in the driver and one in each regmap. Reduce that to one driver only
> > lock that protects all regmap and regcache accesses.
>
> Right. But please consider converting the driver to use cleanup.h.
> Dunno if it requires a separate patch or can be folded into this one
> as it seems you anyway touch almost all mutex calls in the code.
> Linus?
I think it's best to add a separate patch for this for bisection,
just right after this one.
Yours,
Linus Walleij
Tue, May 21, 2024 at 08:34:18PM +0300, Andy Shevchenko kirjoitti:
> On Tue, May 21, 2024 at 6:26 PM Patrick Rudolph
> <[email protected]> wrote:
> >
> > Instead of implementing a custom register paging mechanism in
> > the driver use the existing regmap ranges feature.
>
> driver, use
...
> > +static const struct regmap_range_cfg cy8c95x0_ranges[] = {
> > + {
> > + .range_min = CY8C95X0_VIRTUAL,
> > + .range_max = 0, /* Updated at runtime */
>
> This and similar comments are misleading since the data is defined as
> const. Updated --> Filled or alike here and elsewhere.
>
> > + .selector_reg = CY8C95X0_PORTSEL,
> > + .selector_mask = 0x07,
> > + .selector_shift = 0x0,
> > + .window_start = CY8C95X0_INTMASK,
> > + .window_len = MUXED_STRIDE,
> > + }
> > };
...
> > + regcache_cache_only(chip->regmap, true);
> > + regmap_update_bits(chip->regmap, off, mask & val, 0);
> > + regcache_cache_only(chip->regmap, false);
>
> A side note: It's strange to see mask & val, 0 in the parameters of
> regmap calls. Perhaps refactor this (in a separate patch) to use
> standard approach?
...
> > + memcpy(®map_range_conf, &cy8c95x0_ranges[0], sizeof(regmap_range_conf));
>
> memcpy(®map_range_conf, cy8c95x0_ranges, sizeof(regmap_range_conf));
I hope the all above can be tweaked by Linus when applying.
...
> Note, if you are not using --histogram diff algo, please start using
> it from the next version of this series.
>
> P.S. I will test the next version on Intel Galileo Gen1 as currently I
> have some issues with the HW I need to fix first.
Unfortunately I wasn't able to ressurect the HW and now I'm going to be off for
a while. Let's go with this and if any problem appears, I probably can try to
fix myself.
Reviewed-by: Andy Shevchenko <[email protected]>
--
With Best Regards,
Andy Shevchenko
On Tue, May 21, 2024 at 5:26 PM Patrick Rudolph
<[email protected]> wrote:
> Currently there are 3 locks being used when accessing the chip, one
> in the driver and one in each regmap. Reduce that to one driver only
> lock that protects all regmap and regcache accesses.
>
> Signed-off-by: Patrick Rudolph <[email protected]>
All three patches applied, thanks!
(Looking forward to a <linux/cleanup.h> patch!)
Yours,
Linus Walleij