[PATCH net-next v3 1/2] net: thunderbolt: Switch from __maybe_unused to pm_sleep_ptr() etc

[PATCH net-next v3 1/2] net: thunderbolt: Switch from __maybe_unused to pm_sleep_ptr() etc

[Andy Shevchenko]

Letting the compiler remove these functions when the kernel is built
without CONFIG_PM_SLEEP support is simpler and less heavier for builds
than the use of __maybe_unused attributes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
v3: sent proper patch
v2: added tag (Mika)
 drivers/net/thunderbolt.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
index b20cd370b7f2..c73d419f1456 100644
--- a/drivers/net/thunderbolt.c
+++ b/drivers/net/thunderbolt.c
@@ -1319,7 +1319,7 @@ static void tbnet_shutdown(struct tb_service *svc)
 	tbnet_tear_down(tb_service_get_drvdata(svc), true);
 }
 
-static int __maybe_unused tbnet_suspend(struct device *dev)
+static int tbnet_suspend(struct device *dev)
 {
 	struct tb_service *svc = tb_to_service(dev);
 	struct tbnet *net = tb_service_get_drvdata(svc);
@@ -1334,7 +1334,7 @@ static int __maybe_unused tbnet_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused tbnet_resume(struct device *dev)
+static int tbnet_resume(struct device *dev)
 {
 	struct tb_service *svc = tb_to_service(dev);
 	struct tbnet *net = tb_service_get_drvdata(svc);
@@ -1350,9 +1350,7 @@ static int __maybe_unused tbnet_resume(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops tbnet_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(tbnet_suspend, tbnet_resume)
-};
+static DEFINE_SIMPLE_DEV_PM_OPS(tbnet_pm_ops, tbnet_suspend, tbnet_resume);
 
 static const struct tb_service_id tbnet_ids[] = {
 	{ TB_SERVICE("network", 1) },
@@ -1364,7 +1362,7 @@ static struct tb_service_driver tbnet_driver = {
 	.driver = {
 		.owner = THIS_MODULE,
 		.name = "thunderbolt-net",
-		.pm = &tbnet_pm_ops,
+		.pm = pm_sleep_ptr(&tbnet_pm_ops),
 	},
 	.probe = tbnet_probe,
 	.remove = tbnet_remove,
-- 
2.35.1

[PATCH net-next v3 2/2] net: thunderbolt: Use bitwise types in the struct thunderbolt_ip_frame_header

[Andy Shevchenko]

The main usage of the struct thunderbolt_ip_frame_header is to handle
the packets on the media layer. The header is bound to the protocol
in which the byte ordering is crucial. However the data type definition
doesn't use that and sparse is unhappy, for example (17 altogether):

  .../thunderbolt.c:718:23: warning: cast to restricted __le32

  .../thunderbolt.c:966:42: warning: incorrect type in assignment (different base types)
  .../thunderbolt.c:966:42:    expected unsigned int [usertype] frame_count
  .../thunderbolt.c:966:42:    got restricted __le32 [usertype]

Switch to the bitwise types in the struct thunderbolt_ip_frame_header to
reduce this, but not completely solving (9 left), because the same data
type is used for Rx header handled locally (in CPU byte order).

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: no changes
v2: changed only types without splitting the data type (Mika)
 drivers/net/thunderbolt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
index c73d419f1456..4ed7f5b547e3 100644
--- a/drivers/net/thunderbolt.c
+++ b/drivers/net/thunderbolt.c
@@ -58,10 +58,10 @@
  * supported then @frame_id is filled, otherwise it stays %0.
  */
 struct thunderbolt_ip_frame_header {
-	u32 frame_size;
-	u16 frame_index;
-	u16 frame_id;
-	u32 frame_count;
+	__le32 frame_size;
+	__le16 frame_index;
+	__le16 frame_id;
+	__le32 frame_count;
 };
 
 enum thunderbolt_ip_frame_pdf {
-- 
2.35.1

Re: [PATCH net-next v3 2/2] net: thunderbolt: Use bitwise types in the struct thunderbolt_ip_frame_header

[Mika Westerberg]

On Wed, Nov 30, 2022 at 02:36:13PM +0200, Andy Shevchenko wrote:
> The main usage of the struct thunderbolt_ip_frame_header is to handle
> the packets on the media layer. The header is bound to the protocol
> in which the byte ordering is crucial. However the data type definition
> doesn't use that and sparse is unhappy, for example (17 altogether):
> 
>   .../thunderbolt.c:718:23: warning: cast to restricted __le32
> 
>   .../thunderbolt.c:966:42: warning: incorrect type in assignment (different base types)
>   .../thunderbolt.c:966:42:    expected unsigned int [usertype] frame_count
>   .../thunderbolt.c:966:42:    got restricted __le32 [usertype]
> 
> Switch to the bitwise types in the struct thunderbolt_ip_frame_header to
> reduce this, but not completely solving (9 left), because the same data
> type is used for Rx header handled locally (in CPU byte order).
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Looks good to me. I assume you tested this against non-Linux OS to
ensure nothing broke? ;-)

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH net-next v3 2/2] net: thunderbolt: Use bitwise types in the struct thunderbolt_ip_frame_header

[Andy Shevchenko]

On Wed, Nov 30, 2022 at 02:58:31PM +0200, Mika Westerberg wrote:
> On Wed, Nov 30, 2022 at 02:36:13PM +0200, Andy Shevchenko wrote:
> > The main usage of the struct thunderbolt_ip_frame_header is to handle
> > the packets on the media layer. The header is bound to the protocol
> > in which the byte ordering is crucial. However the data type definition
> > doesn't use that and sparse is unhappy, for example (17 altogether):
> > 
> >   .../thunderbolt.c:718:23: warning: cast to restricted __le32
> > 
> >   .../thunderbolt.c:966:42: warning: incorrect type in assignment (different base types)
> >   .../thunderbolt.c:966:42:    expected unsigned int [usertype] frame_count
> >   .../thunderbolt.c:966:42:    got restricted __le32 [usertype]
> > 
> > Switch to the bitwise types in the struct thunderbolt_ip_frame_header to
> > reduce this, but not completely solving (9 left), because the same data
> > type is used for Rx header handled locally (in CPU byte order).
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Looks good to me. I assume you tested this against non-Linux OS to
> ensure nothing broke? ;-)

Oh, no. It's compile tested only. And since we are using leXX_to_cpu() against
fields in that data structure I assume that it won't be any functional issue
with this. It's all about strict type checking.

> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thank you!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH net-next v3 1/2] net: thunderbolt: Switch from __maybe_unused to pm_sleep_ptr() etc

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed, 30 Nov 2022 14:36:12 +0200 you wrote:
> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less heavier for builds
> than the use of __maybe_unused attributes.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/2] net: thunderbolt: Switch from __maybe_unused to pm_sleep_ptr() etc
    https://git.kernel.org/netdev/net-next/c/0bbe50f3e85a
  - [net-next,v3,2/2] net: thunderbolt: Use bitwise types in the struct thunderbolt_ip_frame_header
    https://git.kernel.org/netdev/net-next/c/a479f9264bdd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html