drivers/iio/chemical/scd30_i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scd30_i2c_cmd_lookup_tbl contains fixed opcodes and is
only read by scd30_i2c_command(). Make it const to document that it's immutable
and allow it to be placed in read-only memory.
Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>
---
drivers/iio/chemical/scd30_i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c
index 436df9c61a71..bf465cc71be7 100644
--- a/drivers/iio/chemical/scd30_i2c.c
+++ b/drivers/iio/chemical/scd30_i2c.c
@@ -20,7 +20,7 @@
#define SCD30_I2C_MAX_BUF_SIZE 18
#define SCD30_I2C_CRC8_POLYNOMIAL 0x31
-static u16 scd30_i2c_cmd_lookup_tbl[] = {
+static const u16 scd30_i2c_cmd_lookup_tbl[] = {
[CMD_START_MEAS] = 0x0010,
[CMD_STOP_MEAS] = 0x0104,
[CMD_MEAS_INTERVAL] = 0x4600,
--
2.52.0
scd30_i2c_cmd_lookup_tbl[] holds the fixed CMD_* opcodes and is only ever read in scd30_i2c_command() (via put_unaligned_be16 of the looked up entry). Marking it const matches how it is used and lets the table land in .rodata. Reviewed-by: Stepan Ionichev <sozdayvek@gmail.com> Stepan
On Sun, 10 May 2026, Andy Shevchenko wrote: > > scd30_i2c_cmd_lookup_tbl[] holds the fixed CMD_* opcodes and is only > > ever read in scd30_i2c_command() (via put_unaligned_be16 of the looked > > up entry). Marking it const matches how it is used and lets the table > > land in .rodata. > > I believe the above is the suggestion on the correction of the commit message? > Please, make it clear! Sorry for the ambiguity -- that paragraph was just my own reasoning for the Reviewed-by tag (i.e. what I checked and why I think the change is correct), not a proposed rewording of the commit message. Giorgi's commit message already reads fine; no changes requested from me. Stepan
On Sun, 10 May 2026, Maxwell Doose wrote: > I'm curious, are you having AI do your reviews? English is not my native language, so I use AI to help with phrasing and to learn kernel review style. The technical analysis I do myself -- for this patch I checked via grep that scd30_i2c_cmd_lookup_tbl[] is only read (one read site in scd30_i2c_command() via put_unaligned_be16, no writes), which is what supports the const change being correct. Happy to follow whatever disclosure norm the iio community prefers. Stepan
Thanks, makes sense. I'll stick with just the R-b tag for the next ones. Stepan
On Sun, May 10, 2026 at 2:59 AM Stepan Ionichev <sozdayvek@gmail.com> wrote: > > On Sun, 10 May 2026, Maxwell Doose wrote: > > I'm curious, are you having AI do your reviews? > > English is not my native language, so I use AI to help with phrasing > and to learn kernel review style. The technical analysis I do myself > -- for this patch I checked via grep that scd30_i2c_cmd_lookup_tbl[] > is only read (one read site in scd30_i2c_command() via > put_unaligned_be16, no writes), which is what supports the const > change being correct. > Thanks for confirming, just wanted to check since some of the phrasing in your review did seem very AI-like. > > Happy to follow whatever disclosure norm the iio community prefers. > Typically reviews end up being one-liners (e.g., Reviewed-by: name <email>) and that ends up being the whole message, sometimes reviewers and maintainers will have inline comments. Obviously you don't have to follow those norms but that's just what usually ends up happening. best regards, max > Stepan
On Sun, May 10, 2026 at 03:04:28AM -0500, Maxwell Doose wrote: > On Sun, May 10, 2026 at 2:59 AM Stepan Ionichev <sozdayvek@gmail.com> wrote: > > On Sun, 10 May 2026, Maxwell Doose wrote: > > > I'm curious, are you having AI do your reviews? > > > > English is not my native language, so I use AI to help with phrasing > > and to learn kernel review style. The technical analysis I do myself > > -- for this patch I checked via grep that scd30_i2c_cmd_lookup_tbl[] > > is only read (one read site in scd30_i2c_command() via > > put_unaligned_be16, no writes), which is what supports the const > > change being correct. > > Thanks for confirming, just wanted to check since some of the phrasing > in your review did seem very AI-like. > > > Happy to follow whatever disclosure norm the iio community prefers. > > Typically reviews end up being one-liners (e.g., Reviewed-by: name > <email>) and that ends up being the whole message, sometimes reviewers > and maintainers will have inline comments. Obviously you don't have to > follow those norms but that's just what usually ends up happening. The good review includes reasoning, and Stepan's is a good one, just needed clarification, because I haven't got if it's a proposal to have a commit message changed or summary of the review. > > Stepan -- With Best Regards, Andy Shevchenko
On Sun, May 10, 2026 at 2:47 AM Stepan Ionichev <sozdayvek@gmail.com> wrote: > > On Sun, 10 May 2026, Andy Shevchenko wrote: > > > scd30_i2c_cmd_lookup_tbl[] holds the fixed CMD_* opcodes and is only > > > ever read in scd30_i2c_command() (via put_unaligned_be16 of the looked > > > up entry). Marking it const matches how it is used and lets the table > > > land in .rodata. > > > > I believe the above is the suggestion on the correction of the commit message? > > Please, make it clear! > > Sorry for the ambiguity -- that paragraph was just my own reasoning > for the Reviewed-by tag (i.e. what I checked and why I think the > change is correct), not a proposed rewording of the commit message. > Giorgi's commit message already reads fine; no changes requested > from me. > I'm curious, are you having AI do your reviews? best regards, max
On Sat, May 09, 2026 at 04:16:33PM +0500, Stepan Ionichev wrote: > scd30_i2c_cmd_lookup_tbl[] holds the fixed CMD_* opcodes and is only > ever read in scd30_i2c_command() (via put_unaligned_be16 of the looked > up entry). Marking it const matches how it is used and lets the table > land in .rodata. I believe the above is the suggestion on the correction of the commit message? Please, make it clear! > Reviewed-by: Stepan Ionichev <sozdayvek@gmail.com> -- With Best Regards, Andy Shevchenko
On Fri, 8 May 2026 17:39:17 +0400
Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com> wrote:
> scd30_i2c_cmd_lookup_tbl contains fixed opcodes and is
> only read by scd30_i2c_command(). Make it const to document that it's immutable
> and allow it to be placed in read-only memory.
>
> Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com>
Applied to the testing branch of iio.git. Thanks,
Jonathan
> ---
> drivers/iio/chemical/scd30_i2c.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c
> index 436df9c61a71..bf465cc71be7 100644
> --- a/drivers/iio/chemical/scd30_i2c.c
> +++ b/drivers/iio/chemical/scd30_i2c.c
> @@ -20,7 +20,7 @@
> #define SCD30_I2C_MAX_BUF_SIZE 18
> #define SCD30_I2C_CRC8_POLYNOMIAL 0x31
>
> -static u16 scd30_i2c_cmd_lookup_tbl[] = {
> +static const u16 scd30_i2c_cmd_lookup_tbl[] = {
> [CMD_START_MEAS] = 0x0010,
> [CMD_STOP_MEAS] = 0x0104,
> [CMD_MEAS_INTERVAL] = 0x4600,
On Fri, May 8, 2026 at 8:53 AM Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com> wrote: > > scd30_i2c_cmd_lookup_tbl contains fixed opcodes and is > only read by scd30_i2c_command(). Make it const to document that it's immutable > and allow it to be placed in read-only memory. > > Signed-off-by: Giorgi Tchankvetadze <giorgitchankvetadze1997@gmail.com> > --- > drivers/iio/chemical/scd30_i2c.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > [snip] Looks fine to me. Acked-by: Maxwell Doose <m32285159@gmail.com> best regards, max
© 2016 - 2026 Red Hat, Inc.