[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8d5a2647-a130-4d99-a3e1-3abd1d336bbb@gmail.com>
Date: Sun, 12 Jan 2025 01:04:34 +0200
From: Markuss Broks <markuss.broks@...il.com>
To: nekodevelopper@...il.com, Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH 3/4] iio: accel: mc3230: add mc3510c support
On 1/11/25 10:11 PM, Vasiliy Doylov via B4 Relay wrote:
> From: Vasiliy Doylov <nekodevelopper@...il.com>
>
> This commit integrates support for the mc3510c into the mc3230 driver.
>
> Tested on Huawei MediaPad T3 10 (huawei-agassi)
>
> Signed-off-by: Vasiliy Doylov <nekodevelopper@...il.com>
> ---
> drivers/iio/accel/mc3230.c | 55 ++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c
> index 3cad6f2d7a2a79df38f90e5656763f6ed019a920..ebbb96c658d87a83007c7c3c7212ce9ebf039963 100644
> --- a/drivers/iio/accel/mc3230.c
> +++ b/drivers/iio/accel/mc3230.c
> @@ -22,20 +22,41 @@
> #define MC3230_MODE_OPCON_STANDBY 0x03
>
> #define MC3230_REG_CHIP_ID 0x18
> -#define MC3230_CHIP_ID 0x01
> -
> #define MC3230_REG_PRODUCT_CODE 0x3b
> -#define MC3230_PRODUCT_CODE 0x19
>
> /*
> * The accelerometer has one measurement range:
> *
> * -1.5g - +1.5g (8-bit, signed)
> *
> - * scale = (1.5 + 1.5) * 9.81 / (2^8 - 1) = 0.115411765
> */
>
> -static const int mc3230_nscale = 115411765;
> +enum mc3xxx_chips {
> + MC3230,
> + MC3510C,
> +};
> +
> +struct mc3xxx_chip_info {
> + const char *name;
> + const u8 chip_id;
> + const u8 product_code;
> + const int scale;
> +};
The struct members are usually ordered alphabetically. Also, const
specifiers for u8s and int are redundant, you will only want it for the
pointer, usually.
> +
> +static struct mc3xxx_chip_info mc3xxx_chip_info_tbl[] = {
> + [MC3230] = {
> + .name = "mc3230",
> + .chip_id = 0x01,
> + .product_code = 0x19,
> + .scale = 115411765, // (1.5 + 1.5) * 9.81 / (2^8 - 1) = 0.115411765
/* */ style comments are preferred. Also, it should be above the .scale
to not make the line so long (even if the line length requirement is met).
> + },
> + [MC3510C] = {
> + .name = "mc3510c",
> + .chip_id = 0x23,
> + .product_code = 0x10,
> + .scale = 625000000, // Was obtained empirically
Same here.
> + },
> +};
>
> #define MC3230_CHANNEL(reg, axis) { \
> .type = IIO_ACCEL, \
> @@ -50,6 +71,7 @@ static const int mc3230_nscale = 115411765;
> struct mc3230_data {
> struct i2c_client *client;
> struct iio_mount_matrix orientation;
> + const struct mc3xxx_chip_info *chip_info;
Same here, order alphabetically.
> };
>
> static const struct iio_mount_matrix *
> @@ -111,7 +133,7 @@ static int mc3230_read_raw(struct iio_dev *indio_dev,
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> *val = 0;
> - *val2 = mc3230_nscale;
> + *val2 = data->chip_info->scale;
> return IIO_VAL_INT_PLUS_NANO;
> default:
> return -EINVAL;
> @@ -127,15 +149,23 @@ static int mc3230_probe(struct i2c_client *client)
> int ret;
> struct iio_dev *indio_dev;
> struct mc3230_data *data;
> + const struct mc3xxx_chip_info *chip_info;
>
> + chip_info = i2c_get_match_data(client);
> /* First check chip-id and product-id */
> ret = i2c_smbus_read_byte_data(client, MC3230_REG_CHIP_ID);
> - if (ret != MC3230_CHIP_ID)
> + if (ret != chip_info->chip_id) {
> + dev_err(&client->dev,
> + "chip id check fail: 0x%x != 0x%x !\n", ret, chip_info->chip_id);
> return (ret < 0) ? ret : -ENODEV;
> + }
>
> ret = i2c_smbus_read_byte_data(client, MC3230_REG_PRODUCT_CODE);
> - if (ret != MC3230_PRODUCT_CODE)
> + if (ret != chip_info->product_code) {
> + dev_err(&client->dev,
> + "product code check fail: 0x%x != 0x%x !\n", ret, chip_info->product_code);
> return (ret < 0) ? ret : -ENODEV;
> + }
>
> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> if (!indio_dev) {
> @@ -145,10 +175,11 @@ static int mc3230_probe(struct i2c_client *client)
>
> data = iio_priv(indio_dev);
> data->client = client;
> + data->chip_info = chip_info;
> i2c_set_clientdata(client, indio_dev);
>
> indio_dev->info = &mc3230_info;
> - indio_dev->name = "mc3230";
> + indio_dev->name = chip_info->name;
> indio_dev->modes = INDIO_DIRECT_MODE;
> indio_dev->channels = mc3230_channels;
> indio_dev->num_channels = ARRAY_SIZE(mc3230_channels);
> @@ -200,13 +231,15 @@ static int mc3230_resume(struct device *dev)
> static DEFINE_SIMPLE_DEV_PM_OPS(mc3230_pm_ops, mc3230_suspend, mc3230_resume);
>
> static const struct i2c_device_id mc3230_i2c_id[] = {
> - { "mc3230" },
> + { "mc3230", (kernel_ulong_t)&mc3xxx_chip_info_tbl[MC3230] },
> + { "mc3510c", (kernel_ulong_t)&mc3xxx_chip_info_tbl[MC3510C] },
> {}
> };
> MODULE_DEVICE_TABLE(i2c, mc3230_i2c_id);
>
> static const struct of_device_id mc3230_of_match[] = {
> - { .compatible = "mcube,mc3230" },
> + { .compatible = "mcube,mc3230", &mc3xxx_chip_info_tbl[MC3230] },
> + { .compatible = "mcube,mc3510c", &mc3xxx_chip_info_tbl[MC3510C] },
> { },
> };
> MODULE_DEVICE_TABLE(of, mc3230_of_match);
Powered by blists - more mailing lists