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 <[email protected]>
---
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 a52ee2bf5575..4dbc6c7f2e10 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
The same data type structure is used for bitwise operations and
regular ones. It makes sparse unhappy, for example:
.../thunderbolt.c:718:23: warning: cast to restricted __le32
.../thunderbolt.c:953:23: warning: incorrect type in initializer (different base types)
.../thunderbolt.c:953:23: expected restricted __wsum [usertype] wsum
.../thunderbolt.c:953:23: got restricted __be32 [usertype]
Split the header to bitwise one and specific for Rx to make sparse
happy. Assure the layout by involving static_assert() against size
and offsets of the member of the structures.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/net/thunderbolt.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
index 4dbc6c7f2e10..f7b3d0d4646c 100644
--- a/drivers/net/thunderbolt.c
+++ b/drivers/net/thunderbolt.c
@@ -58,12 +58,32 @@
* supported then @frame_id is filled, otherwise it stays %0.
*/
struct thunderbolt_ip_frame_header {
+ __le32 frame_size;
+ __le16 frame_index;
+ __le16 frame_id;
+ __le32 frame_count;
+};
+
+/* Same as &struct thunderbolt_ip_frame_header for Rx */
+struct thunderbolt_ip_frame_rx_hdr {
u32 frame_size;
u16 frame_index;
u16 frame_id;
u32 frame_count;
};
+static_assert(sizeof(struct thunderbolt_ip_frame_header) ==
+ sizeof(struct thunderbolt_ip_frame_rx_hdr));
+
+#define TBIP_FRAME_HDR_MATCH(x) \
+ static_assert(offsetof(struct thunderbolt_ip_frame_header, frame_##x) == \
+ offsetof(struct thunderbolt_ip_frame_rx_hdr, frame_##x))
+TBIP_FRAME_HDR_MATCH(size);
+TBIP_FRAME_HDR_MATCH(index);
+TBIP_FRAME_HDR_MATCH(id);
+TBIP_FRAME_HDR_MATCH(count);
+#undef TBIP_FRAME_HDR_MATCH
+
enum thunderbolt_ip_frame_pdf {
TBIP_PDF_FRAME_START = 1,
TBIP_PDF_FRAME_END,
@@ -193,7 +213,7 @@ struct tbnet {
struct delayed_work login_work;
struct work_struct connected_work;
struct work_struct disconnect_work;
- struct thunderbolt_ip_frame_header rx_hdr;
+ struct thunderbolt_ip_frame_rx_hdr rx_hdr;
struct tbnet_ring rx_ring;
atomic_t frame_id;
struct tbnet_ring tx_ring;
--
2.35.1
On Tue, Nov 29, 2022 at 06:13:59PM +0200, Andy Shevchenko wrote:
> The same data type structure is used for bitwise operations and
> regular ones. It makes sparse unhappy, for example:
>
> .../thunderbolt.c:718:23: warning: cast to restricted __le32
>
> .../thunderbolt.c:953:23: warning: incorrect type in initializer (different base types)
> .../thunderbolt.c:953:23: expected restricted __wsum [usertype] wsum
> .../thunderbolt.c:953:23: got restricted __be32 [usertype]
>
> Split the header to bitwise one and specific for Rx to make sparse
> happy. Assure the layout by involving static_assert() against size
> and offsets of the member of the structures.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/net/thunderbolt.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
I would much rather keep the humans reading this happy than add 20+
lines just to silence a tool. Unless this of course is some kind of a
real bug.
On Tue, Nov 29, 2022 at 06:13:58PM +0200, Andy Shevchenko 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 <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
On Wed, Nov 30, 2022 at 09:46:16AM +0200, Mika Westerberg wrote:
> On Tue, Nov 29, 2022 at 06:13:59PM +0200, Andy Shevchenko wrote:
> > The same data type structure is used for bitwise operations and
> > regular ones. It makes sparse unhappy, for example:
> >
> > .../thunderbolt.c:718:23: warning: cast to restricted __le32
> >
> > .../thunderbolt.c:953:23: warning: incorrect type in initializer (different base types)
> > .../thunderbolt.c:953:23: expected restricted __wsum [usertype] wsum
> > .../thunderbolt.c:953:23: got restricted __be32 [usertype]
> >
> > Split the header to bitwise one and specific for Rx to make sparse
> > happy. Assure the layout by involving static_assert() against size
> > and offsets of the member of the structures.
> I would much rather keep the humans reading this happy than add 20+
> lines just to silence a tool. Unless this of course is some kind of a
> real bug.
Actually, changing types to bitwise ones reduces the sparse noise
(I will double check this) without reducing readability.
Would it be accepted?
--
With Best Regards,
Andy Shevchenko
On Wed, Nov 30, 2022 at 12:51:06PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 30, 2022 at 09:46:16AM +0200, Mika Westerberg wrote:
> > On Tue, Nov 29, 2022 at 06:13:59PM +0200, Andy Shevchenko wrote:
> > > The same data type structure is used for bitwise operations and
> > > regular ones. It makes sparse unhappy, for example:
> > >
> > > .../thunderbolt.c:718:23: warning: cast to restricted __le32
> > >
> > > .../thunderbolt.c:953:23: warning: incorrect type in initializer (different base types)
> > > .../thunderbolt.c:953:23: expected restricted __wsum [usertype] wsum
> > > .../thunderbolt.c:953:23: got restricted __be32 [usertype]
> > >
> > > Split the header to bitwise one and specific for Rx to make sparse
> > > happy. Assure the layout by involving static_assert() against size
> > > and offsets of the member of the structures.
>
> > I would much rather keep the humans reading this happy than add 20+
> > lines just to silence a tool. Unless this of course is some kind of a
> > real bug.
>
> Actually, changing types to bitwise ones reduces the sparse noise
> (I will double check this) without reducing readability.
> Would it be accepted?
Sure if it makes it more readable and does not add too many lines :)
On Wed, Nov 30, 2022 at 01:09:59PM +0200, Mika Westerberg wrote:
> On Wed, Nov 30, 2022 at 12:51:06PM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 30, 2022 at 09:46:16AM +0200, Mika Westerberg wrote:
> > > On Tue, Nov 29, 2022 at 06:13:59PM +0200, Andy Shevchenko wrote:
> > > > The same data type structure is used for bitwise operations and
> > > > regular ones. It makes sparse unhappy, for example:
> > > >
> > > > .../thunderbolt.c:718:23: warning: cast to restricted __le32
> > > >
> > > > .../thunderbolt.c:953:23: warning: incorrect type in initializer (different base types)
> > > > .../thunderbolt.c:953:23: expected restricted __wsum [usertype] wsum
> > > > .../thunderbolt.c:953:23: got restricted __be32 [usertype]
> > > >
> > > > Split the header to bitwise one and specific for Rx to make sparse
> > > > happy. Assure the layout by involving static_assert() against size
> > > > and offsets of the member of the structures.
> >
> > > I would much rather keep the humans reading this happy than add 20+
> > > lines just to silence a tool. Unless this of course is some kind of a
> > > real bug.
> >
> > Actually, changing types to bitwise ones reduces the sparse noise
> > (I will double check this) without reducing readability.
> > Would it be accepted?
>
> Sure if it makes it more readable and does not add too many lines :)
It replaces types u* by __le*, that's it: -4 +4 LoCs.
--
With Best Regards,
Andy Shevchenko