Introduce an enumeration to define PSE types (C33 or PoDL),
utilizing a bitfield for potential future support of both types.
Include 'pse_get_types' helper for external access to PSE type info.
This patch is sponsored by Dent Project <[email protected]>.
Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: Kory Maincent <[email protected]>
---
Changes in v2:
- Rename PSE_POE to PSE_C33 to have naming consistency.
- Use "static inline" instead of simple static in the header
Changes in v3:
- Move the pse_type enum in uapi.
- Replace pse_get_types helper by pse_has_podl and pse_has_c33.
---
drivers/net/pse-pd/pse_core.c | 12 ++++++++++++
drivers/net/pse-pd/pse_regulator.c | 1 +
include/linux/pse-pd/pse.h | 16 ++++++++++++++++
include/uapi/linux/pse.h | 23 +++++++++++++++++++++++
4 files changed, 52 insertions(+)
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 146b81f08a89..090e04c32f9e 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -312,3 +312,15 @@ int pse_ethtool_set_config(struct pse_control *psec,
return err;
}
EXPORT_SYMBOL_GPL(pse_ethtool_set_config);
+
+bool pse_has_podl(struct pse_control *psec)
+{
+ return psec->pcdev->types & PSE_PODL;
+}
+EXPORT_SYMBOL_GPL(pse_has_podl);
+
+bool pse_has_c33(struct pse_control *psec)
+{
+ return psec->pcdev->types & PSE_C33;
+}
+EXPORT_SYMBOL_GPL(pse_has_c33);
diff --git a/drivers/net/pse-pd/pse_regulator.c b/drivers/net/pse-pd/pse_regulator.c
index 1dedf4de296e..e34ab8526067 100644
--- a/drivers/net/pse-pd/pse_regulator.c
+++ b/drivers/net/pse-pd/pse_regulator.c
@@ -116,6 +116,7 @@ pse_reg_probe(struct platform_device *pdev)
priv->pcdev.owner = THIS_MODULE;
priv->pcdev.ops = &pse_reg_ops;
priv->pcdev.dev = dev;
+ priv->pcdev.types = PSE_PODL;
ret = devm_pse_controller_register(dev, &priv->pcdev);
if (ret) {
dev_err(dev, "failed to register PSE controller (%pe)\n",
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index be4e5754eb24..f006cbdf8b3b 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -8,6 +8,7 @@
#include <linux/ethtool.h>
#include <linux/list.h>
#include <uapi/linux/ethtool.h>
+#include <uapi/linux/pse.h>
struct phy_device;
struct pse_controller_dev;
@@ -77,6 +78,7 @@ struct pse_control;
* device tree to id as given to the PSE control ops
* @nr_lines: number of PSE controls in this controller device
* @lock: Mutex for serialization access to the PSE controller
+ * @types: types of the PSE controller
*/
struct pse_controller_dev {
const struct pse_controller_ops *ops;
@@ -89,6 +91,7 @@ struct pse_controller_dev {
const struct of_phandle_args *pse_spec);
unsigned int nr_lines;
struct mutex lock;
+ u32 types;
};
#if IS_ENABLED(CONFIG_PSE_CONTROLLER)
@@ -108,6 +111,9 @@ int pse_ethtool_set_config(struct pse_control *psec,
struct netlink_ext_ack *extack,
const struct pse_control_config *config);
+bool pse_has_podl(struct pse_control *psec);
+bool pse_has_c33(struct pse_control *psec);
+
#else
static inline struct pse_control *of_pse_control_get(struct device_node *node)
@@ -133,6 +139,16 @@ static inline int pse_ethtool_set_config(struct pse_control *psec,
return -ENOTSUPP;
}
+static inline bool pse_has_podl(struct pse_control *psec)
+{
+ return false;
+}
+
+static inline bool pse_has_c33(struct pse_control *psec)
+{
+ return false;
+}
+
#endif
#endif
diff --git a/include/uapi/linux/pse.h b/include/uapi/linux/pse.h
new file mode 100644
index 000000000000..ebd9b4be9d9d
--- /dev/null
+++ b/include/uapi/linux/pse.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Userspace API for Power Sourcing Equipment
+ *
+ * Copyright (c) 2023 Bootlin, Kory Maincent <[email protected]>
+ */
+#ifndef _PSE_CONTROLLER_H
+#define _PSE_CONTROLLER_H
+
+/**
+ * enum - Types of PSE controller.
+ *
+ * @PSE_UNKNOWN: Type of PSE controller is unknown
+ * @PSE_PODL: PSE controller which support PoDL
+ * @PSE_C33: PSE controller which support Clause 33 (PoE)
+ */
+enum {
+ PSE_UNKNOWN = 1 << 0,
+ PSE_PODL = 1 << 1,
+ PSE_C33 = 1 << 2,
+};
+
+#endif /* _PSE_CONTROLLER_H */
--
2.25.1
On Thu, 15 Feb 2024 17:02:46 +0100 Kory Maincent wrote:
> Introduce an enumeration to define PSE types (C33 or PoDL),
> utilizing a bitfield for potential future support of both types.
> Include 'pse_get_types' helper for external access to PSE type info.
I haven't read the series, just noticed this breaks the build:
error: ../include/uapi/linux/pse.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
but why the separate header? Is it going to be used in other parts of
uAPI than just in ethtool?
> This patch is sponsored by Dent Project <[email protected]>
side-note: no objections to the line but for accounting purposes
(i.e. when we generate development stats) we use the Author / From
line exclusively. So it'd be easier to compute stats of things funded
by Dent if you used:
From: Kory Maincent (Dent Project) <[email protected]>
but that's entirely up to you :)
On Thu, 15 Feb 2024 10:58:46 -0800
Jakub Kicinski <[email protected]> wrote:
> On Thu, 15 Feb 2024 17:02:46 +0100 Kory Maincent wrote:
> > Introduce an enumeration to define PSE types (C33 or PoDL),
> > utilizing a bitfield for potential future support of both types.
> > Include 'pse_get_types' helper for external access to PSE type info.
>
> I haven't read the series, just noticed this breaks the build:
>
> error: ../include/uapi/linux/pse.h: missing "WITH Linux-syscall-note" for
> SPDX-License-Identifier
By curiosity how do you get that error?
Is it with C=1? I didn't faced it with W=1.
C=1 is broken for several architecture like arm64, indeed I forgot to run it.
> but why the separate header? Is it going to be used in other parts of
> uAPI than just in ethtool?
We might use it in pse core if capabilities between PoE and PoDL differ but I
am not sure about it.
Do you prefer to move it to ethtool header and add prefix ETHTOOL_ to the enum
values?
> > This patch is sponsored by Dent Project <[email protected]>
>
> side-note: no objections to the line but for accounting purposes
> (i.e. when we generate development stats) we use the Author / From
> line exclusively. So it'd be easier to compute stats of things funded
> by Dent if you used:
>
> From: Kory Maincent (Dent Project) <[email protected]>
>
> but that's entirely up to you :)
Does adding the line side to the SOB in the commit message is sufficient or
should I modify the git send email config?
Regard,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
On Fri, 16 Feb 2024 10:42:11 +0100 Köry Maincent wrote:
> > On Thu, 15 Feb 2024 17:02:46 +0100 Kory Maincent wrote:
> > > Introduce an enumeration to define PSE types (C33 or PoDL),
> > > utilizing a bitfield for potential future support of both types.
> > > Include 'pse_get_types' helper for external access to PSE type info.
> >
> > I haven't read the series, just noticed this breaks the build:
> >
> > error: ../include/uapi/linux/pse.h: missing "WITH Linux-syscall-note" for
> > SPDX-License-Identifier
>
> By curiosity how do you get that error?
> Is it with C=1? I didn't faced it with W=1.
> C=1 is broken for several architecture like arm64, indeed I forgot to run it.
Not 100% sure, TBH, I suspect it's somehow enabled by allmodconfig.
I don't think it's a C=1 thing because our clang build doesn't do C=1
and it also hit it.
> > but why the separate header? Is it going to be used in other parts of
> > uAPI than just in ethtool?
>
> We might use it in pse core if capabilities between PoE and PoDL differ but I
> am not sure about it.
> Do you prefer to move it to ethtool header and add prefix ETHTOOL_ to the enum
> values?
I don't know enough to have an opinion :) Whatever you end up doing,
it's probably worth documenting the reason for the choice in the commit
message?
> > > This patch is sponsored by Dent Project <[email protected]>
> >
> > side-note: no objections to the line but for accounting purposes
> > (i.e. when we generate development stats) we use the Author / From
> > line exclusively. So it'd be easier to compute stats of things funded
> > by Dent if you used:
> >
> > From: Kory Maincent (Dent Project) <[email protected]>
> >
> > but that's entirely up to you :)
>
> Does adding the line side to the SOB in the commit message is sufficient or
> should I modify the git send email config?
I think you can sed -i s/// the patches? When the From in the email
file doesn't match your git config IIUC git will include the from line
in the body and pick it up from them. IOW it will work. The scripts look
at git author so s-o-b won't do much.
On Fri, 16 Feb 2024 17:36:38 -0800
Jakub Kicinski <[email protected]> wrote:
> > > but why the separate header? Is it going to be used in other parts of
> > > uAPI than just in ethtool?
> >
> > We might use it in pse core if capabilities between PoE and PoDL differ but
> > I am not sure about it.
> > Do you prefer to move it to ethtool header and add prefix ETHTOOL_ to the
> > enum values?
>
> I don't know enough to have an opinion :) Whatever you end up doing,
> it's probably worth documenting the reason for the choice in the commit
> message?
Mmh, I am still not sure of the best choice on this. I think I will move it to
ethtool as you suggested.
> > > > This patch is sponsored by Dent Project
> > > > <[email protected]>
> > >
> > > side-note: no objections to the line but for accounting purposes
> > > (i.e. when we generate development stats) we use the Author / From
> > > line exclusively. So it'd be easier to compute stats of things funded
> > > by Dent if you used:
> > >
> > > From: Kory Maincent (Dent Project) <[email protected]>
> > >
> > > but that's entirely up to you :)
> >
> > Does adding the line side to the SOB in the commit message is sufficient or
> > should I modify the git send email config?
>
> I think you can sed -i s/// the patches? When the From in the email
> file doesn't match your git config IIUC git will include the from line
> in the body and pick it up from them. IOW it will work. The scripts look
> at git author so s-o-b won't do much.
Ok, I will stick to the simple sentence then. ;)
Thanks for the information!
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
On Mon, Feb 19, 2024 at 04:04:56PM +0100, K?ry Maincent wrote:
> On Fri, 16 Feb 2024 17:36:38 -0800
> Jakub Kicinski <[email protected]> wrote:
> > > > but why the separate header? Is it going to be used in other parts of
> > > > uAPI than just in ethtool?
> > >
> > > We might use it in pse core if capabilities between PoE and PoDL differ but
> > > I am not sure about it.
> > > Do you prefer to move it to ethtool header and add prefix ETHTOOL_ to the
> > > enum values?
> >
> > I don't know enough to have an opinion :) Whatever you end up doing,
> > it's probably worth documenting the reason for the choice in the commit
> > message?
>
> Mmh, I am still not sure of the best choice on this. I think I will move it to
> ethtool as you suggested.
kAPI is hard to change. So it is worth thinking about it.
Can you think of any possible kAPI not using ethtool netlink? Its not
going to be ioctl. We generally don't export new things in /sysfs if
we have netlink, etc.
So to me, it is only going to be used be the ethtool API, so i would
follow the usual conventions for ethtool.
Andrew
On Fri, Feb 16, 2024 at 05:36:38PM -0800, Jakub Kicinski wrote:
> On Fri, 16 Feb 2024 10:42:11 +0100 K?ry Maincent wrote:
> > > On Thu, 15 Feb 2024 17:02:46 +0100 Kory Maincent wrote:
> > > > Introduce an enumeration to define PSE types (C33 or PoDL),
> > > > utilizing a bitfield for potential future support of both types.
> > > > Include 'pse_get_types' helper for external access to PSE type info.
> > >
> > > I haven't read the series, just noticed this breaks the build:
> > >
> > > error: ../include/uapi/linux/pse.h: missing "WITH Linux-syscall-note" for
> > > SPDX-License-Identifier
> >
> > By curiosity how do you get that error?
> > Is it with C=1? I didn't faced it with W=1.
> > C=1 is broken for several architecture like arm64, indeed I forgot to run it.
>
> Not 100% sure, TBH, I suspect it's somehow enabled by allmodconfig.
> I don't think it's a C=1 thing because our clang build doesn't do C=1
> and it also hit it.
Probably from scripts/spdxcheck.py? IIRC, it has some dependencies which
have to be installed to enable it.
Rob
On Mon, 19 Feb 2024 16:44:34 +0100
Andrew Lunn <[email protected]> wrote:
> On Mon, Feb 19, 2024 at 04:04:56PM +0100, Köry Maincent wrote:
> > On Fri, 16 Feb 2024 17:36:38 -0800
> > Jakub Kicinski <[email protected]> wrote:
> > > > > but why the separate header? Is it going to be used in other parts of
> > > > > uAPI than just in ethtool?
> > > >
> > > > We might use it in pse core if capabilities between PoE and PoDL differ
> > > > but I am not sure about it.
> > > > Do you prefer to move it to ethtool header and add prefix ETHTOOL_ to
> > > > the enum values?
> > >
> > > I don't know enough to have an opinion :) Whatever you end up doing,
> > > it's probably worth documenting the reason for the choice in the commit
> > > message?
> >
> > Mmh, I am still not sure of the best choice on this. I think I will move it
> > to ethtool as you suggested.
>
> kAPI is hard to change. So it is worth thinking about it.
>
> Can you think of any possible kAPI not using ethtool netlink? Its not
> going to be ioctl. We generally don't export new things in /sysfs if
> we have netlink, etc.
>
> So to me, it is only going to be used be the ethtool API, so i would
> follow the usual conventions for ethtool.
Oops sorry forgot to reply to you.
Indeed I reached to the same conclusion on my side.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com