[PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

Subsystems: linux for power macintosh, the rest

STALE2628d LANDED

Landed in mainline as 7440ea8b2a44 on 2019-07-31.

8 messages, 5 authors, 2019-08-02 · open the first message on its own page

[PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Stephen Rothwell <hidden>
Date: 2019-07-30 04:37:21

Mark switch cases where we are expecting to fall through.

This patch fixes the following warning (Building: powerpc):

drivers/macintosh/smu.c: In function 'smu_queue_i2c':
drivers/macintosh/smu.c:854:21: warning: this statement may fall through [-Wimplicit-fallthrough=]
   cmd->info.devaddr &= 0xfe;
   ~~~~~~~~~~~~~~~~~~^~~~~~~
drivers/macintosh/smu.c:855:2: note: here
  case SMU_I2C_TRANSFER_STDSUB:
  ^~~~

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Gustavo A. R. Silva <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Stephen Rothwell <redacted>
---
 drivers/macintosh/smu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
index 276065c888bc..23f1f41c8602 100644
--- a/drivers/macintosh/smu.c
+++ b/drivers/macintosh/smu.c

@@ -852,6 +852,7 @@ int smu_queue_i2c(struct smu_i2c_cmd *cmd)
 		break;
 	case SMU_I2C_TRANSFER_COMBINED:
 		cmd->info.devaddr &= 0xfe;
+		/* fall through */
 	case SMU_I2C_TRANSFER_STDSUB:
 		if (cmd->info.sublen > 3)
 			return -EINVAL;

-- 
2.22.0

-- 
Cheers,
Stephen Rothwell

Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Stephen Rothwell <hidden>
Date: 2019-07-30 04:45:30

Hi all,

On Tue, 30 Jul 2019 14:37:04 +1000 Stephen Rothwell [off-list ref] wrote:

Mark switch cases where we are expecting to fall through.

This patch fixes the following warning (Building: powerpc):

drivers/macintosh/smu.c: In function 'smu_queue_i2c':
drivers/macintosh/smu.c:854:21: warning: this statement may fall through [-Wimplicit-fallthrough=]
   cmd->info.devaddr &= 0xfe;
   ~~~~~~~~~~~~~~~~~~^~~~~~~
drivers/macintosh/smu.c:855:2: note: here
  case SMU_I2C_TRANSFER_STDSUB:
  ^~~~

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Gustavo A. R. Silva <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Stephen Rothwell <redacted>

Fixes: 0365ba7fb1fa ("[PATCH] ppc64: SMU driver update & i2c support")

Sorry, forgot :-)
-- 
Cheers,
Stephen Rothwell

Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2019-07-30 14:29:05

Stephen Rothwell [off-list ref] writes:

quoted hunk

Mark switch cases where we are expecting to fall through.

This patch fixes the following warning (Building: powerpc):

drivers/macintosh/smu.c: In function 'smu_queue_i2c':
drivers/macintosh/smu.c:854:21: warning: this statement may fall through [-Wimplicit-fallthrough=]
   cmd->info.devaddr &= 0xfe;
   ~~~~~~~~~~~~~~~~~~^~~~~~~
drivers/macintosh/smu.c:855:2: note: here
  case SMU_I2C_TRANSFER_STDSUB:
  ^~~~

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Gustavo A. R. Silva <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Stephen Rothwell <redacted>
---
 drivers/macintosh/smu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
index 276065c888bc..23f1f41c8602 100644
--- a/drivers/macintosh/smu.c
+++ b/drivers/macintosh/smu.c

@@ -852,6 +852,7 @@ int smu_queue_i2c(struct smu_i2c_cmd *cmd)
 		break;
 	case SMU_I2C_TRANSFER_COMBINED:
 		cmd->info.devaddr &= 0xfe;
+		/* fall through */
 	case SMU_I2C_TRANSFER_STDSUB:
 		if (cmd->info.sublen > 3)
 			return -EINVAL;

Why do we think it's an expected fall through? I can't really convince
myself from the surrounding code that it's definitely intentional.

cheers

Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Stephen Rothwell <hidden>
Date: 2019-07-30 14:42:13

Hi Michael,

On Wed, 31 Jul 2019 00:28:55 +1000 Michael Ellerman [off-list ref] wrote:

Why do we think it's an expected fall through? I can't really convince
myself from the surrounding code that it's definitely intentional.

Its been that way since this code was introduced by commit

  0365ba7fb1fa ("[PATCH] ppc64: SMU driver update & i2c support")

in 2005 ...

-- 
Cheers,
Stephen Rothwell

Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Kees Cook <hidden>
Date: 2019-07-30 17:07:53

On Wed, Jul 31, 2019 at 12:28:55AM +1000, Michael Ellerman wrote:

Stephen Rothwell [off-list ref] writes:

quoted

Mark switch cases where we are expecting to fall through.

This patch fixes the following warning (Building: powerpc):

drivers/macintosh/smu.c: In function 'smu_queue_i2c':
drivers/macintosh/smu.c:854:21: warning: this statement may fall through [-Wimplicit-fallthrough=]
   cmd->info.devaddr &= 0xfe;
   ~~~~~~~~~~~~~~~~~~^~~~~~~
drivers/macintosh/smu.c:855:2: note: here
  case SMU_I2C_TRANSFER_STDSUB:
  ^~~~

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Gustavo A. R. Silva <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Stephen Rothwell <redacted>
---
 drivers/macintosh/smu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
index 276065c888bc..23f1f41c8602 100644
--- a/drivers/macintosh/smu.c
+++ b/drivers/macintosh/smu.c

@@ -852,6 +852,7 @@ int smu_queue_i2c(struct smu_i2c_cmd *cmd)
 		break;
 	case SMU_I2C_TRANSFER_COMBINED:
 		cmd->info.devaddr &= 0xfe;
+		/* fall through */
 	case SMU_I2C_TRANSFER_STDSUB:
 		if (cmd->info.sublen > 3)
 			return -EINVAL;

Why do we think it's an expected fall through? I can't really convince
myself from the surrounding code that it's definitely intentional.

Yeah, good question. Just now when I went looking for who
used SMU_I2C_TRANSFER_COMBINED, I found the only caller in
arch/powerpc/platforms/powermac/low_i2c.c and it is clearly using a
fall-through for building the command for "stdsub" and "combined",
so I think that's justification enough:

        switch(bus->mode) {
        case pmac_i2c_mode_std:
                if (subsize != 0)
                        return -EINVAL;
                cmd->info.type = SMU_I2C_TRANSFER_SIMPLE;
                break;
        case pmac_i2c_mode_stdsub:
        case pmac_i2c_mode_combined:
                if (subsize > 3 || subsize < 1)
                        return -EINVAL;
                cmd->info.sublen = subsize;
                /* that's big-endian only but heh ! */
                memcpy(&cmd->info.subaddr, ((char *)&subaddr) + (4 - subsize),
                       subsize);
                if (bus->mode == pmac_i2c_mode_stdsub)
                        cmd->info.type = SMU_I2C_TRANSFER_STDSUB;
                else
                        cmd->info.type = SMU_I2C_TRANSFER_COMBINED;


Reviewed-by: Kees Cook <redacted>

-- 
Kees Cook

Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2019-07-30 20:38:16

On Tue, 2019-07-30 at 10:07 -0700, Kees Cook wrote:

quoted

Why do we think it's an expected fall through? I can't really
convince
myself from the surrounding code that it's definitely intentional.

Yeah, good question. Just now when I went looking for who
used SMU_I2C_TRANSFER_COMBINED, I found the only caller in
arch/powerpc/platforms/powermac/low_i2c.c and it is clearly using a
fall-through for building the command for "stdsub" and "combined",
so I think that's justification enough:

Yes, sorry for the delay, the fall through is intentional.

Cheers,
Ben.

Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2019-07-31 11:46:40

Kees Cook [off-list ref] writes:

On Wed, Jul 31, 2019 at 12:28:55AM +1000, Michael Ellerman wrote:

quoted

Stephen Rothwell [off-list ref] writes:

quoted

Mark switch cases where we are expecting to fall through.

This patch fixes the following warning (Building: powerpc):

drivers/macintosh/smu.c: In function 'smu_queue_i2c':
drivers/macintosh/smu.c:854:21: warning: this statement may fall through [-Wimplicit-fallthrough=]
   cmd->info.devaddr &= 0xfe;
   ~~~~~~~~~~~~~~~~~~^~~~~~~
drivers/macintosh/smu.c:855:2: note: here
  case SMU_I2C_TRANSFER_STDSUB:
  ^~~~

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Gustavo A. R. Silva <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Stephen Rothwell <redacted>
---
 drivers/macintosh/smu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/macintosh/smu.c b/drivers/macintosh/smu.c
index 276065c888bc..23f1f41c8602 100644
--- a/drivers/macintosh/smu.c
+++ b/drivers/macintosh/smu.c

@@ -852,6 +852,7 @@ int smu_queue_i2c(struct smu_i2c_cmd *cmd)
 		break;
 	case SMU_I2C_TRANSFER_COMBINED:
 		cmd->info.devaddr &= 0xfe;
+		/* fall through */
 	case SMU_I2C_TRANSFER_STDSUB:
 		if (cmd->info.sublen > 3)
 			return -EINVAL;

Why do we think it's an expected fall through? I can't really convince
myself from the surrounding code that it's definitely intentional.

Yeah, good question. Just now when I went looking for who
used SMU_I2C_TRANSFER_COMBINED, I found the only caller in
arch/powerpc/platforms/powermac/low_i2c.c and it is clearly using a
fall-through for building the command for "stdsub" and "combined",
so I think that's justification enough:

        switch(bus->mode) {
        case pmac_i2c_mode_std:
                if (subsize != 0)
                        return -EINVAL;
                cmd->info.type = SMU_I2C_TRANSFER_SIMPLE;
                break;
        case pmac_i2c_mode_stdsub:
        case pmac_i2c_mode_combined:
                if (subsize > 3 || subsize < 1)
                        return -EINVAL;
                cmd->info.sublen = subsize;
                /* that's big-endian only but heh ! */
                memcpy(&cmd->info.subaddr, ((char *)&subaddr) + (4 - subsize),
                       subsize);
                if (bus->mode == pmac_i2c_mode_stdsub)
                        cmd->info.type = SMU_I2C_TRANSFER_STDSUB;
                else
                        cmd->info.type = SMU_I2C_TRANSFER_COMBINED;


Reviewed-by: Kees Cook <redacted>

Thanks.

cheers

Re: [PATCH] drivers/macintosh/smu.c: Mark expected switch fall-through

From: Michael Ellerman <hidden>
Date: 2019-08-02 02:28:31

On Tue, 2019-07-30 at 04:37:04 UTC, Stephen Rothwell wrote:

Mark switch cases where we are expecting to fall through.

This patch fixes the following warning (Building: powerpc):

drivers/macintosh/smu.c: In function 'smu_queue_i2c':
drivers/macintosh/smu.c:854:21: warning: this statement may fall through [-=
Wimplicit-fallthrough=3D]
   cmd->info.devaddr &=3D 0xfe;
   ~~~~~~~~~~~~~~~~~~^~~~~~~
drivers/macintosh/smu.c:855:2: note: here
  case SMU_I2C_TRANSFER_STDSUB:
  ^~~~

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Gustavo A. R. Silva <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Stephen Rothwell <redacted>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/7440ea8b2a4430eef5120d0a7faac6c39304ae6d

cheers

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help