Hi,
Thanks Lee for your review!
On Fri, 2025-04-04 at 10:18 +0100, Lee Jones wrote:
> On Thu, 03 Apr 2025, André Draszik wrote:
[...]
> > diff --git a/drivers/mfd/sec-acpm.c b/drivers/mfd/sec-acpm.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..39dbb968086ac835b96ed3e4efa68868fda63429
> > --- /dev/null
> > +++ b/drivers/mfd/sec-acpm.c
> > @@ -0,0 +1,465 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright 2020 Google Inc
> > + * Copyright 2025 Linaro Ltd.
> > + *
> > + * Samsung S2MPG1x ACPM driver
> > + */
> > +
> > +#include <linux/array_size.h>
> > +#include <linux/device.h>
> > +#include <linux/firmware/samsung/exynos-acpm-protocol.h>
> > +#include <linux/mfd/samsung/core.h>
> > +#include <linux/mfd/samsung/rtc.h>
> > +#include <linux/mfd/samsung/s2mpg10.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +#include "sec-core.h"
> > +
> > +#define ACPM_MAX_BULK_DATA 8
> > +
> > +struct sec_pmic_acpm_platform_data {
>
> This isn't platform data. It's driver data.
>
> Platform data is passed in, driver data is derived.
This is the match data from of_device_id::data, it is passed in
via device_get_match_data().
>
> See how the other drivers do this:
>
> $ git grep ddata -- drivers/mfd
I had followed the example of drivers/mfd/rk8xx-i2c.c
I can rename it to struct sec_pmic_acpm_chip_data if you prefer or
something like that, but the rk8xx driver also calls this platform
data.
ddata in drivers/mfd/ generally seems used for dynamically allocated
runtime driver data. That's not the case here.
> > + int device_type;
> > +
> > + unsigned int acpm_chan_id;
> > + u8 speedy_channel;
> > +
> > + const struct regmap_config *regmap_cfg_common;
> > + const struct regmap_config *regmap_cfg_pmic;
> > + const struct regmap_config *regmap_cfg_rtc;
> > + const struct regmap_config *regmap_cfg_meter;
> > +};
> > +
> > +static const struct regmap_range s2mpg10_common_registers[] = {
> > + regmap_reg_range(0x00, 0x02), /* CHIP_ID_M, INT, INT_MASK */
> > + regmap_reg_range(0x0a, 0x0c), /* speedy control */
> > + regmap_reg_range(0x1a, 0x2a), /* debug */
>
> Nit: I like comments to start with an upper-case char.
OK
>
> > +};
> > +
> > +static const struct regmap_range s2mpg10_common_ro_registers[] = {
> > + regmap_reg_range(0x00, 0x01),
> > + regmap_reg_range(0x28, 0x2a),
>
> Why describe some, but not all ranges?
They're all covered above. I'll duplicate them here and elsewhere.
> > +struct sec_pmic_acpm_shared_bus_context {
> > + const struct acpm_handle *acpm;
> > + unsigned int acpm_chan_id;
> > + u8 speedy_channel;
> > +};
> > +
> > +enum sec_pmic_acpm_accesstype {
> > + SEC_PMIC_ACPM_ACCESSTYPE_COMMON = 0x00,
> > + SEC_PMIC_ACPM_ACCESSTYPE_PMIC = 0x01,
> > + SEC_PMIC_ACPM_ACCESSTYPE_RTC = 0x02,
> > + SEC_PMIC_ACPM_ACCESSTYPE_METER = 0x0a,
> > + SEC_PMIC_ACPM_ACCESSTYPE_WLWP = 0x0b,
> > + SEC_PMIC_ACPM_ACCESSTYPE_TRIM = 0x0f,
> > +};
> > +
> > +struct sec_pmic_acpm_bus_context {
> > + struct sec_pmic_acpm_shared_bus_context *shared;
> > + enum sec_pmic_acpm_accesstype type;
> > +};
> > +
> > +static int sec_pmic_acpm_bus_write(void *context, const void *data,
> > + size_t count)
>
> Nit: You can tidy this, and similar line-feeds, up by using 100-chars here.
Will do.
> > +{
> > + struct sec_pmic_acpm_bus_context *ctx = context;
> > + const struct acpm_handle *acpm = ctx->shared->acpm;
> > + const struct acpm_pmic_ops *pmic_ops = &acpm->ops.pmic_ops;
> > + u8 reg;
> > +
> > + if (count < 2 || count > (ACPM_MAX_BULK_DATA + 1))
>
> 2 because? Either comment or define magic numbers please.
>
> > + return -EINVAL;
> > +
> > + reg = *(u8 *)data;
>
> No API to conduct this raw read for you? readl(), *_to_cpu() or similar?
This is just regmap, passing a buffer. First byte(s) contains the reg
address, depending on the regmap_config used during creation, and remainder
the values starting from that address. This is not an I/O read as such, it's
only extracting the register address. See e.g. regmap_parse_8().
I'll reflow it a little.
>
> > + ++data;
> > + --count;
> > +
> > + return pmic_ops->bulk_write(acpm, ctx->shared->acpm_chan_id,
> > + ctx->type, reg,
> > + ctx->shared->speedy_channel, count, data);
> > +}
> > +
> > +static int sec_pmic_acpm_bus_read(void *context, const void *reg_buf,
> > + size_t reg_size, void *val_buf,
> > + size_t val_size)
> > +{
> > + struct sec_pmic_acpm_bus_context *ctx = context;
> > + const struct acpm_handle *acpm = ctx->shared->acpm;
> > + const struct acpm_pmic_ops *pmic_ops = &acpm->ops.pmic_ops;
> > + u8 reg;
> > +
> > + if (reg_size != 1 || !val_size || val_size > ACPM_MAX_BULK_DATA)
> > + return -EINVAL;
> > +
> > + reg = *(u8 *)reg_buf;
> > +
> > + return pmic_ops->bulk_read(acpm, ctx->shared->acpm_chan_id,
> > + ctx->type, reg,
> > + ctx->shared->speedy_channel,
> > + val_size, val_buf);
> > +}
> > +
> > +static int sec_pmic_acpm_bus_reg_update_bits(void *context, unsigned int reg,
> > + unsigned int mask,
> > + unsigned int val)
> > +{
> > + struct sec_pmic_acpm_bus_context *ctx = context;
> > + const struct acpm_handle *acpm = ctx->shared->acpm;
> > + const struct acpm_pmic_ops *pmic_ops = &acpm->ops.pmic_ops;
> > +
> > + return pmic_ops->update_reg(acpm, ctx->shared->acpm_chan_id,
> > + ctx->type, reg & 0xff,
> > + ctx->shared->speedy_channel, val, mask);
> > +}
> > +
> > +static const struct regmap_bus sec_pmic_acpm_regmap_bus = {
> > + .write = sec_pmic_acpm_bus_write,
> > + .read = sec_pmic_acpm_bus_read,
> > + .reg_update_bits = sec_pmic_acpm_bus_reg_update_bits,
> > + .max_raw_read = ACPM_MAX_BULK_DATA,
> > + .max_raw_write = ACPM_MAX_BULK_DATA,
> > +};
> > +
> > +static struct regmap *
> > +sec_pmic_acpm_regmap_init(struct device *dev,
>
> Place both of these on the same line please.
>
> > + struct sec_pmic_acpm_shared_bus_context *shared_ctx,
> > + enum sec_pmic_acpm_accesstype type,
> > + const struct regmap_config *cfg, bool do_attach)
> > +{
> > + struct sec_pmic_acpm_bus_context *ctx;
> > + struct regmap *regmap;
> > +
> > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + ctx->shared = shared_ctx;
> > + ctx->type = type;
> > +
> > + regmap = devm_regmap_init(dev, &sec_pmic_acpm_regmap_bus, ctx, cfg);
> > + if (IS_ERR(regmap))
> > + return ERR_PTR(dev_err_probe(dev, PTR_ERR(regmap),
>
> dev_err_cast_probe()
>
> > + "regmap init (%s) failed\n",
> > + cfg->name));
> > +
> > + if (do_attach) {
> > + int ret;
> > +
> > + ret = regmap_attach_dev(dev, regmap, cfg);
> > + if (ret)
> > + return ERR_PTR(dev_err_probe(dev, ret,
>
> dev_err_ptr_probe()
Thanks! I had forgotten about those two.
> > + "regmap attach (%s) failed\n",
> > + cfg->name));
> > + }
> > +
> > + return regmap;
> > +}
> > +
> > +static void sec_pmic_acpm_mask_common_irqs(void *regmap_common)
> > +{
> > + regmap_write(regmap_common, S2MPG10_COMMON_INT_MASK,
> > + S2MPG10_COMMON_INT_SRC);
>
> Single line. And others like it.
>
> > +}
> > +
> > +static int sec_pmic_acpm_probe(struct platform_device *pdev)
> > +{
> > + struct regmap *regmap_common, *regmap_pmic, *regmap;
> > + const struct sec_pmic_acpm_platform_data *pdata;
> > + struct sec_pmic_acpm_shared_bus_context *shared_ctx;
> > + const struct acpm_handle *acpm;
> > + struct device *dev;
> > + int ret, irq;
> > +
> > + dev = &pdev->dev;
>
> You can do this during the declaration.
>
> > + pdata = device_get_match_data(dev);
> > + if (!pdata)
> > + return dev_err_probe(dev, -ENODEV,
> > + "unsupported device type\n");
> > +
> > + acpm = devm_acpm_get_by_node(dev, pdev->dev.parent->of_node);
>
> You have 'dev' now. Please use it.
>
> > + if (IS_ERR(acpm))
> > + return dev_err_probe(dev, PTR_ERR(acpm),
> > + "failed to get acpm (2)\n");
> > +
> > + irq = platform_get_irq(pdev, 0);
> > + if (irq < 0)
> > + return irq;
> > +
> > + shared_ctx = devm_kzalloc(dev, sizeof(*shared_ctx), GFP_KERNEL);
> > + if (!shared_ctx)
> > + return -ENOMEM;
> > +
> > + shared_ctx->acpm = acpm;
> > + shared_ctx->acpm_chan_id = pdata->acpm_chan_id;
> > + shared_ctx->speedy_channel = pdata->speedy_channel;
> > +
> > + regmap_common = sec_pmic_acpm_regmap_init(dev, shared_ctx,
> > + SEC_PMIC_ACPM_ACCESSTYPE_COMMON,
> > + pdata->regmap_cfg_common, false);
> > + if (IS_ERR(regmap_common))
> > + return PTR_ERR(regmap_common);
> > +
> > + /* Mask all interrupts from 'common' block, until successful init */
> > + ret = regmap_write(regmap_common, S2MPG10_COMMON_INT_MASK,
> > + S2MPG10_COMMON_INT_SRC);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "failed to mask common block interrupts\n");
> > +
> > + regmap_pmic = sec_pmic_acpm_regmap_init(dev, shared_ctx,
> > + SEC_PMIC_ACPM_ACCESSTYPE_PMIC,
> > + pdata->regmap_cfg_pmic, false);
> > + if (IS_ERR(regmap_pmic))
> > + return PTR_ERR(regmap_pmic);
> > +
> > + regmap = sec_pmic_acpm_regmap_init(dev, shared_ctx,
> > + SEC_PMIC_ACPM_ACCESSTYPE_RTC,
> > + pdata->regmap_cfg_rtc, true);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > + regmap = sec_pmic_acpm_regmap_init(dev, shared_ctx,
> > + SEC_PMIC_ACPM_ACCESSTYPE_METER,
> > + pdata->regmap_cfg_meter, true);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > + ret = sec_pmic_probe(dev, pdata->device_type, irq, regmap_pmic, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + if (device_property_read_bool(dev, "wakeup-source"))
> > + devm_device_init_wakeup(dev);
> > +
> > + /*
> > + * Unmask PMIC interrupt from 'common' block, now that everything is in
> > + * place.
> > + */
> > + ret = regmap_clear_bits(regmap_common, S2MPG10_COMMON_INT_MASK,
> > + S2MPG10_COMMON_INT_SRC_PMIC);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "failed to unmask PMIC interrupt\n");
> > +
> > + /* Mask all interrupts from 'common' block on shutdown */
> > + ret = devm_add_action_or_reset(dev, sec_pmic_acpm_mask_common_irqs,
> > + regmap_common);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static void sec_pmic_acpm_shutdown(struct platform_device *pdev)
> > +{
> > + sec_pmic_shutdown(&pdev->dev);
>
> sec_pmic_shutdown() takes a pointer to i2c_client (unless you changed it
> somewhere else). If the later is true, then why not make it take a
> pointer to platform_device and omit this abstraction?
I changed it earlier indeed to support both I2C and ACPM transports, similar
to drivers/mfd/rk*. The I2C driver doesn't have a struct platform_device,
but it has struct i2c_client::dev, hence I'm passing struct device to the
common code, like in the rk8xx example.
[...]
>
> > diff --git a/drivers/mfd/sec-irq.c b/drivers/mfd/sec-irq.c
> > index 4d49bb42bd0d109263f485c8b58e88cdd8d598d9..bf86281401ac6ff05c90c2d71c84744709ed79cb 100644
> > --- a/drivers/mfd/sec-irq.c
> > +++ b/drivers/mfd/sec-irq.c
> > @@ -11,6 +11,7 @@
> > #include <linux/irq.h>
> > #include <linux/mfd/samsung/core.h>
> > #include <linux/mfd/samsung/irq.h>
> > +#include <linux/mfd/samsung/s2mpg10.h>
> > #include <linux/mfd/samsung/s2mps11.h>
> > #include <linux/mfd/samsung/s2mps14.h>
> > #include <linux/mfd/samsung/s2mpu02.h>
> > @@ -20,6 +21,60 @@
> > #include <linux/regmap.h>
> > #include "sec-core.h"
> >
> > +static const struct regmap_irq s2mpg10_irqs[] = {
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWRONF, 0, S2MPG10_IRQ_PWRONF_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWRONR, 0, S2MPG10_IRQ_PWRONR_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_JIGONBF, 0, S2MPG10_IRQ_JIGONBF_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_JIGONBR, 0, S2MPG10_IRQ_JIGONBR_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_ACOKBF, 0, S2MPG10_IRQ_ACOKBF_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_ACOKBR, 0, S2MPG10_IRQ_ACOKBR_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWRON1S, 0, S2MPG10_IRQ_PWRON1S_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_MRB, 0, S2MPG10_IRQ_MRB_MASK),
> > +
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_RTC60S, 1, S2MPG10_IRQ_RTC60S_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_RTCA1, 1, S2MPG10_IRQ_RTCA1_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_RTCA0, 1, S2MPG10_IRQ_RTCA0_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_RTC1S, 1, S2MPG10_IRQ_RTC1S_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_WTSR_COLDRST, 1, S2MPG10_IRQ_WTSR_COLDRST_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_WTSR, 1, S2MPG10_IRQ_WTSR_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_WRST, 1, S2MPG10_IRQ_WRST_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_SMPL, 1, S2MPG10_IRQ_SMPL_MASK),
> > +
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_120C, 2, S2MPG10_IRQ_INT120C_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_140C, 2, S2MPG10_IRQ_INT140C_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_TSD, 2, S2MPG10_IRQ_TSD_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PIF_TIMEOUT1, 2, S2MPG10_IRQ_PIF_TIMEOUT1_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PIF_TIMEOUT2, 2, S2MPG10_IRQ_PIF_TIMEOUT2_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_SPD_PARITY_ERR, 2, S2MPG10_IRQ_SPD_PARITY_ERR_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_SPD_ABNORMAL_STOP, 2, S2MPG10_IRQ_SPD_ABNORMAL_STOP_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PMETER_OVERF, 2, S2MPG10_IRQ_PMETER_OVERF_MASK),
> > +
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B1M, 3, S2MPG10_IRQ_OCP_B1M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B2M, 3, S2MPG10_IRQ_OCP_B2M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B3M, 3, S2MPG10_IRQ_OCP_B3M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B4M, 3, S2MPG10_IRQ_OCP_B4M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B5M, 3, S2MPG10_IRQ_OCP_B5M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B6M, 3, S2MPG10_IRQ_OCP_B6M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B7M, 3, S2MPG10_IRQ_OCP_B7M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B8M, 3, S2MPG10_IRQ_OCP_B8M_MASK),
> > +
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B9M, 4, S2MPG10_IRQ_OCP_B9M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_OCP_B10M, 4, S2MPG10_IRQ_OCP_B10M_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_WLWP_ACC, 4, S2MPG10_IRQ_WLWP_ACC_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_SMPL_TIMEOUT, 4, S2MPG10_IRQ_SMPL_TIMEOUT_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_WTSR_TIMEOUT, 4, S2MPG10_IRQ_WTSR_TIMEOUT_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_SPD_SRP_PKT_RST, 4, S2MPG10_IRQ_SPD_SRP_PKT_RST_MASK),
> > +
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH0, 5, S2MPG10_IRQ_PWR_WARN_CH0_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH1, 5, S2MPG10_IRQ_PWR_WARN_CH1_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH2, 5, S2MPG10_IRQ_PWR_WARN_CH2_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH3, 5, S2MPG10_IRQ_PWR_WARN_CH3_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH4, 5, S2MPG10_IRQ_PWR_WARN_CH4_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH5, 5, S2MPG10_IRQ_PWR_WARN_CH5_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH6, 5, S2MPG10_IRQ_PWR_WARN_CH6_MASK),
> > + REGMAP_IRQ_REG(S2MPG10_IRQ_PWR_WARN_CH7, 5, S2MPG10_IRQ_PWR_WARN_CH7_MASK),
> > +};
> > +
> > static const struct regmap_irq s2mps11_irqs[] = {
> > [S2MPS11_IRQ_PWRONF] = {
> > .reg_offset = 0,
> > @@ -320,6 +375,16 @@ static const struct regmap_irq s5m8767_irqs[] = {
> > },
> > };
> >
> > +static const struct regmap_irq_chip s2mpg10_irq_chip = {
> > + .name = "s2mpg10",
> > + .irqs = s2mpg10_irqs,
> > + .num_irqs = ARRAY_SIZE(s2mpg10_irqs),
> > + .num_regs = 6,
> > + .status_base = S2MPG10_PMIC_INT1,
> > + .mask_base = S2MPG10_PMIC_INT1M,
> > + /* all interrupt sources are read-to-clear */
>
> TOUPPER(a);
>
> Comments usually go on-top of the thing they're commenting on.
This comment is where .ack_base would usually be specified, but I'll move it.
[...]
> > diff --git a/include/linux/mfd/samsung/s2mpg10.h b/include/linux/mfd/samsung/s2mpg10.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..778ff16ef6668ded514e8dc7242f369cb9c2d0e6
> > --- /dev/null
> > +++ b/include/linux/mfd/samsung/s2mpg10.h
> > @@ -0,0 +1,454 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2015 Samsung Electronics
> > + * Copyright 2020 Google Inc
> > + * Copyright 2025 Linaro Ltd.
> > + */
> > +
> > +#ifndef __LINUX_MFD_S2MPG10_H
> > +#define __LINUX_MFD_S2MPG10_H
> > +
> > +/* Common registers (type 0x000) */
> > +enum s2mpg10_common_reg {
> > + S2MPG10_COMMON_CHIPID,
> > + S2MPG10_COMMON_INT,
> > + S2MPG10_COMMON_INT_MASK,
> > + S2MPG10_COMMON_SPD_CTRL1 = 0x0a,
> > + S2MPG10_COMMON_SPD_CTRL2,
> > + S2MPG10_COMMON_SPD_CTRL3,
> > + S2MPG10_COMMON_MON1SEL = 0x1a,
> > + S2MPG10_COMMON_MON2SEL,
> > + S2MPG10_COMMON_MONR,
> > + S2MPG10_COMMON_DEBUG_CTRL1,
> > + S2MPG10_COMMON_DEBUG_CTRL2,
> > + S2MPG10_COMMON_DEBUG_CTRL3,
> > + S2MPG10_COMMON_DEBUG_CTRL4,
> > + S2MPG10_COMMON_DEBUG_CTRL5,
> > + S2MPG10_COMMON_DEBUG_CTRL6,
> > + S2MPG10_COMMON_DEBUG_CTRL7,
> > + S2MPG10_COMMON_DEBUG_CTRL8,
> > + S2MPG10_COMMON_TEST_MODE1,
> > + S2MPG10_COMMON_TEST_MODE2,
> > + S2MPG10_COMMON_SPD_DEBUG1,
> > + S2MPG10_COMMON_SPD_DEBUG2,
> > + S2MPG10_COMMON_SPD_DEBUG3,
> > + S2MPG10_COMMON_SPD_DEBUG4,
> > +};
> > +
> > +/* for S2MPG10_COMMON_INT and S2MPG10_COMMON_INT_MASK */
>
> TOUPPER(f), etc.
Still getting used to this, sorry I missed them
Thanks Lee!
Andre'
