| Message ID | 20220626192444.29321-1-vfedorenko@novek.ru (mailing list archive) |
|---|---|
| Headers | show |
| Series | Create common DPLL/clock configuration API | expand |
On 26/06/2022 22:24, Vadim Fedorenko wrote: > From: Vadim Fedorenko <vadfed@fb.com> > > Implement common API for clock/DPLL configuration and status reporting. > The API utilises netlink interface as transport for commands and event > notifications. This API aim to extend current pin configuration and > make it flexible and easy to cover special configurations. Hello Vadim, I'm trying to understand how we can register our mlx5 hardware with this DPLL subsystem, and honestly I don't understand how this is going to be used eventually. Is there anywhere else I can read more about this work? Maybe some documentation explaining the use-cases? Is there userspace code I can see to get a sense of the full picture? Thanks
Sun, Jun 26, 2022 at 09:24:41PM CEST, vfedorenko@novek.ru wrote: >From: Vadim Fedorenko <vadfed@fb.com> > >Implement common API for clock/DPLL configuration and status reporting. >The API utilises netlink interface as transport for commands and event >notifications. This API aim to extend current pin configuration and >make it flexible and easy to cover special configurations. Do you have the userspace part somewhere? It is very nice to add example outputs of user cmdline of such tool to the patch description/cover letter. Also, did you consider usage of sysfs? Why it isn't a better fit than netlink? Regarding the naming, is "dpll" the correct one. Forgive me for being a syncE greenie, but isn't dpll just one algo to achieve syntonous clocks? Perhaps "dco" as for "Digitally Controlled Oscillator" would be somewhat better fit? > >v1 -> v2: > * implement returning supported input/output types > * ptp_ocp: follow suggestions from Jonathan > * add linux-clk mailing list >v0 -> v1: > * fix code style and errors > * add linux-arm mailing list > > >Vadim Fedorenko (3): > dpll: Add DPLL framework base functions > dpll: add netlink events > ptp_ocp: implement DPLL ops > > MAINTAINERS | 8 + > drivers/Kconfig | 2 + > drivers/Makefile | 1 + > drivers/dpll/Kconfig | 7 + > drivers/dpll/Makefile | 7 + > drivers/dpll/dpll_core.c | 161 ++++++++++ > drivers/dpll/dpll_core.h | 40 +++ > drivers/dpll/dpll_netlink.c | 595 ++++++++++++++++++++++++++++++++++++ > drivers/dpll/dpll_netlink.h | 14 + > drivers/ptp/Kconfig | 1 + > drivers/ptp/ptp_ocp.c | 169 +++++++--- > include/linux/dpll.h | 29 ++ > include/uapi/linux/dpll.h | 81 +++++ > 13 files changed, 1079 insertions(+), 36 deletions(-) > create mode 100644 drivers/dpll/Kconfig > create mode 100644 drivers/dpll/Makefile > create mode 100644 drivers/dpll/dpll_core.c > create mode 100644 drivers/dpll/dpll_core.h > create mode 100644 drivers/dpll/dpll_netlink.c > create mode 100644 drivers/dpll/dpll_netlink.h > create mode 100644 include/linux/dpll.h > create mode 100644 include/uapi/linux/dpll.h > >-- >2.27.0 >
On 29.09.2022 12:40, Jiri Pirko wrote: > Sun, Jun 26, 2022 at 09:24:41PM CEST, vfedorenko@novek.ru wrote: >> From: Vadim Fedorenko <vadfed@fb.com> >> >> Implement common API for clock/DPLL configuration and status reporting. >> The API utilises netlink interface as transport for commands and event >> notifications. This API aim to extend current pin configuration and >> make it flexible and easy to cover special configurations. > > Do you have the userspace part somewhere? > It is very nice to add example outputs of user cmdline of such tool to > the patch description/cover letter. Sorry, but we don't have any user-space part for now. It's still WIP and there are too many changes in the protocol to implement anything useful on top of it. Once we will get to a kind of "stable" proto, I will implement a library to use it. > > Also, did you consider usage of sysfs? Why it isn't a better fit than > netlink? We already have sysfs implemented in the ptp_ocp driver. But it looks like more hardware is going to be available soon with almost the same functions, so it would be great to have common protocol to configure such devices. > > Regarding the naming, is "dpll" the correct one. Forgive me for being a > syncE greenie, but isn't dpll just one algo to achieve syntonous > clocks? Perhaps "dco" as for "Digitally Controlled Oscillator" would be > somewhat better fit? > We will discuss the naming too, thanks! > >> >> v1 -> v2: >> * implement returning supported input/output types >> * ptp_ocp: follow suggestions from Jonathan >> * add linux-clk mailing list >> v0 -> v1: >> * fix code style and errors >> * add linux-arm mailing list >> >> >> Vadim Fedorenko (3): >> dpll: Add DPLL framework base functions >> dpll: add netlink events >> ptp_ocp: implement DPLL ops >> >> MAINTAINERS | 8 + >> drivers/Kconfig | 2 + >> drivers/Makefile | 1 + >> drivers/dpll/Kconfig | 7 + >> drivers/dpll/Makefile | 7 + >> drivers/dpll/dpll_core.c | 161 ++++++++++ >> drivers/dpll/dpll_core.h | 40 +++ >> drivers/dpll/dpll_netlink.c | 595 ++++++++++++++++++++++++++++++++++++ >> drivers/dpll/dpll_netlink.h | 14 + >> drivers/ptp/Kconfig | 1 + >> drivers/ptp/ptp_ocp.c | 169 +++++++--- >> include/linux/dpll.h | 29 ++ >> include/uapi/linux/dpll.h | 81 +++++ >> 13 files changed, 1079 insertions(+), 36 deletions(-) >> create mode 100644 drivers/dpll/Kconfig >> create mode 100644 drivers/dpll/Makefile >> create mode 100644 drivers/dpll/dpll_core.c >> create mode 100644 drivers/dpll/dpll_core.h >> create mode 100644 drivers/dpll/dpll_netlink.c >> create mode 100644 drivers/dpll/dpll_netlink.h >> create mode 100644 include/linux/dpll.h >> create mode 100644 include/uapi/linux/dpll.h >> >> -- >> 2.27.0 >>
Fri, Sep 30, 2022 at 02:44:25AM CEST, vfedorenko@novek.ru wrote: >On 29.09.2022 12:40, Jiri Pirko wrote: >> Sun, Jun 26, 2022 at 09:24:41PM CEST, vfedorenko@novek.ru wrote: >> > From: Vadim Fedorenko <vadfed@fb.com> >> > >> > Implement common API for clock/DPLL configuration and status reporting. >> > The API utilises netlink interface as transport for commands and event >> > notifications. This API aim to extend current pin configuration and >> > make it flexible and easy to cover special configurations. >> >> Do you have the userspace part somewhere? >> It is very nice to add example outputs of user cmdline of such tool to >> the patch description/cover letter. > >Sorry, but we don't have any user-space part for now. It's still WIP and >there are too many changes in the protocol to implement anything useful on What protocol? >top of it. Once we will get to a kind of "stable" proto, I will implement a >library to use it. > >> >> Also, did you consider usage of sysfs? Why it isn't a better fit than >> netlink? > >We already have sysfs implemented in the ptp_ocp driver. But it looks like >more hardware is going to be available soon with almost the same functions, >so it would be great to have common protocol to configure such devices. Sure, but more hw does not mean you can't use sysfs. Take netdev as an example. The sysfs exposed for it is implemented net/core/net-sysfs.c and is exposed for all netdev instances, no matter what the driver/hardware is. >> >> Regarding the naming, is "dpll" the correct one. Forgive me for being a >> syncE greenie, but isn't dpll just one algo to achieve syntonous >> clocks? Perhaps "dco" as for "Digitally Controlled Oscillator" would be >> somewhat better fit? >> > >We will discuss the naming too, thanks! > >> >> > >> > v1 -> v2: >> > * implement returning supported input/output types >> > * ptp_ocp: follow suggestions from Jonathan >> > * add linux-clk mailing list >> > v0 -> v1: >> > * fix code style and errors >> > * add linux-arm mailing list >> > >> > >> > Vadim Fedorenko (3): >> > dpll: Add DPLL framework base functions >> > dpll: add netlink events >> > ptp_ocp: implement DPLL ops >> > >> > MAINTAINERS | 8 + >> > drivers/Kconfig | 2 + >> > drivers/Makefile | 1 + >> > drivers/dpll/Kconfig | 7 + >> > drivers/dpll/Makefile | 7 + >> > drivers/dpll/dpll_core.c | 161 ++++++++++ >> > drivers/dpll/dpll_core.h | 40 +++ >> > drivers/dpll/dpll_netlink.c | 595 ++++++++++++++++++++++++++++++++++++ >> > drivers/dpll/dpll_netlink.h | 14 + >> > drivers/ptp/Kconfig | 1 + >> > drivers/ptp/ptp_ocp.c | 169 +++++++--- >> > include/linux/dpll.h | 29 ++ >> > include/uapi/linux/dpll.h | 81 +++++ >> > 13 files changed, 1079 insertions(+), 36 deletions(-) >> > create mode 100644 drivers/dpll/Kconfig >> > create mode 100644 drivers/dpll/Makefile >> > create mode 100644 drivers/dpll/dpll_core.c >> > create mode 100644 drivers/dpll/dpll_core.h >> > create mode 100644 drivers/dpll/dpll_netlink.c >> > create mode 100644 drivers/dpll/dpll_netlink.h >> > create mode 100644 include/linux/dpll.h >> > create mode 100644 include/uapi/linux/dpll.h >> > >> > -- >> > 2.27.0 >> > >
On Fri, 30 Sep 2022 10:33:57 +0200 Jiri Pirko wrote: > >> Also, did you consider usage of sysfs? Why it isn't a better fit than > >> netlink? > > > >We already have sysfs implemented in the ptp_ocp driver. But it looks like > >more hardware is going to be available soon with almost the same functions, > >so it would be great to have common protocol to configure such devices. > > Sure, but more hw does not mean you can't use sysfs. Take netdev as an > example. The sysfs exposed for it is implemented net/core/net-sysfs.c > and is exposed for all netdev instances, no matter what the > driver/hardware is. Wait, *you* are suggesting someone uses sysfs instead of netlink? Could you say more because I feel like that's kicking the absolute.
Fri, Sep 30, 2022 at 04:33:12PM CEST, kuba@kernel.org wrote: >On Fri, 30 Sep 2022 10:33:57 +0200 Jiri Pirko wrote: >> >> Also, did you consider usage of sysfs? Why it isn't a better fit than >> >> netlink? >> > >> >We already have sysfs implemented in the ptp_ocp driver. But it looks like >> >more hardware is going to be available soon with almost the same functions, >> >so it would be great to have common protocol to configure such devices. >> >> Sure, but more hw does not mean you can't use sysfs. Take netdev as an >> example. The sysfs exposed for it is implemented net/core/net-sysfs.c >> and is exposed for all netdev instances, no matter what the >> driver/hardware is. > >Wait, *you* are suggesting someone uses sysfs instead of netlink? > >Could you say more because I feel like that's kicking the absolute. I don't understand why that would be a problem. What I'm trying to say is, perhaps sysfs is a better API for this purpose. The API looks very neat and there is no probabilito of huge grow. Also, with sysfs, you don't need userspace app to do basic work with the api. In this case, I don't see why the app is needed. These are 2 biggest arguments for sysfs in this case as I see it.
On Sat, 1 Oct 2022 07:47:24 +0200 Jiri Pirko wrote: > >> Sure, but more hw does not mean you can't use sysfs. Take netdev as an > >> example. The sysfs exposed for it is implemented net/core/net-sysfs.c > >> and is exposed for all netdev instances, no matter what the > >> driver/hardware is. > > > >Wait, *you* are suggesting someone uses sysfs instead of netlink? > > > >Could you say more because I feel like that's kicking the absolute. > > I don't understand why that would be a problem. Why did you do devlink over netlink then? The bus device is already there in sysfs. > What I'm trying to say > is, perhaps sysfs is a better API for this purpose. The API looks very > neat and there is no probabilito of huge grow. "this API is nice and small" said everyone about every new API ever, APIs grow. > Also, with sysfs, you > don't need userspace app to do basic work with the api. In this case, I > don't see why the app is needed. Yes, with the YAML specs you don't need a per-family APP. A generic app can support any family, just JSON in JSON out. DPLL-nl will come with a YAML spec. > These are 2 biggest arguments for sysfs in this case as I see it. 2 biggest arguments? Is "this API is small" one of the _biggest_ arguments you see? I don't think it's an argument at all. The OCP PTP driver started small and now its not small. And the files don't even follow sysfs rules. Trust me, we have some experience here :/ As I said to you in private I feel like there may be some political games being played here, so I'd like to urge you to focus on real issues.
Sat, Oct 01, 2022 at 04:18:27PM CEST, kuba@kernel.org wrote: >On Sat, 1 Oct 2022 07:47:24 +0200 Jiri Pirko wrote: >> >> Sure, but more hw does not mean you can't use sysfs. Take netdev as an >> >> example. The sysfs exposed for it is implemented net/core/net-sysfs.c >> >> and is exposed for all netdev instances, no matter what the >> >> driver/hardware is. >> > >> >Wait, *you* are suggesting someone uses sysfs instead of netlink? >> > >> >Could you say more because I feel like that's kicking the absolute. >> >> I don't understand why that would be a problem. > >Why did you do devlink over netlink then? There were good reasons why to use netlink, many of those. I find it redundant to list them here. >The bus device is already there in sysfs. > >> What I'm trying to say >> is, perhaps sysfs is a better API for this purpose. The API looks very >> neat and there is no probabilito of huge grow. > >"this API is nice and small" said everyone about every new API ever, >APIs grow. Sure, what what are the odds. > >> Also, with sysfs, you >> don't need userspace app to do basic work with the api. In this case, I >> don't see why the app is needed. > >Yes, with the YAML specs you don't need a per-family APP. >A generic app can support any family, just JSON in JSON out. >DPLL-nl will come with a YAML spec. Yeah, but still. For sysfs, you don't need any app. Just saying. > >> These are 2 biggest arguments for sysfs in this case as I see it. > >2 biggest arguments? Is "this API is small" one of the _biggest_ >arguments you see? I don't think it's an argument at all. The OCP PTP >driver started small and now its not small. And the files don't even >follow sysfs rules. Trust me, we have some experience here :/ No problem. I don't mind one bit, don't get me wrong :) I just pointed out alternative. > >As I said to you in private I feel like there may be some political >games being played here, so I'd like to urge you to focus on real >issues. I don't know anything about any politics. I don't care about it at all to be honest. You know me :)
On Sun, 2 Oct 2022 16:35:07 +0200 Jiri Pirko wrote: >>> What I'm trying to say >>> is, perhaps sysfs is a better API for this purpose. The API looks very >>> neat and there is no probabilito of huge grow. >> >> "this API is nice and small" said everyone about every new API ever, >> APIs grow. > > Sure, what what are the odds. The pins were made into full objects now, and we also model muxes. Vadim, could you share the link to the GH repo? What's your feeling on posting the latest patches upstream as RFC, whatever state things are in right now? My preference would be to move the development to the list at this stage, FWIW.
On 03.10.2022 15:28, Jakub Kicinski wrote: > On Sun, 2 Oct 2022 16:35:07 +0200 Jiri Pirko wrote: >>>> What I'm trying to say >>>> is, perhaps sysfs is a better API for this purpose. The API looks very >>>> neat and there is no probabilito of huge grow. >>> >>> "this API is nice and small" said everyone about every new API ever, >>> APIs grow. >> >> Sure, what what are the odds. > > The pins were made into full objects now, and we also model muxes. > > Vadim, could you share the link to the GH repo? I've already shared the link to Jiri. > > What's your feeling on posting the latest patches upstream as RFC, > whatever state things are in right now? > I'll post RFC v3 once we agree on locking mechanics for pins. Right now there is no good solution AFAIU, discussions are in progress. > My preference would be to move the development to the list at this > stage, FWIW. I got this point and I will follow it once we will have something ready.
Mon, Oct 03, 2022 at 04:28:31PM CEST, kuba@kernel.org wrote: >On Sun, 2 Oct 2022 16:35:07 +0200 Jiri Pirko wrote: >>>> What I'm trying to say >>>> is, perhaps sysfs is a better API for this purpose. The API looks very >>>> neat and there is no probabilito of huge grow. >>> >>> "this API is nice and small" said everyone about every new API ever, >>> APIs grow. >> >> Sure, what what are the odds. > >The pins were made into full objects now, and we also model muxes. > >Vadim, could you share the link to the GH repo? > >What's your feeling on posting the latest patches upstream as RFC, >whatever state things are in right now? > >My preference would be to move the development to the list at this >stage, FWIW. I agree, that would be great.
From: Vadim Fedorenko <vadfed@fb.com> Implement common API for clock/DPLL configuration and status reporting. The API utilises netlink interface as transport for commands and event notifications. This API aim to extend current pin configuration and make it flexible and easy to cover special configurations. v1 -> v2: * implement returning supported input/output types * ptp_ocp: follow suggestions from Jonathan * add linux-clk mailing list v0 -> v1: * fix code style and errors * add linux-arm mailing list Vadim Fedorenko (3): dpll: Add DPLL framework base functions dpll: add netlink events ptp_ocp: implement DPLL ops MAINTAINERS | 8 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/dpll/Kconfig | 7 + drivers/dpll/Makefile | 7 + drivers/dpll/dpll_core.c | 161 ++++++++++ drivers/dpll/dpll_core.h | 40 +++ drivers/dpll/dpll_netlink.c | 595 ++++++++++++++++++++++++++++++++++++ drivers/dpll/dpll_netlink.h | 14 + drivers/ptp/Kconfig | 1 + drivers/ptp/ptp_ocp.c | 169 +++++++--- include/linux/dpll.h | 29 ++ include/uapi/linux/dpll.h | 81 +++++ 13 files changed, 1079 insertions(+), 36 deletions(-) create mode 100644 drivers/dpll/Kconfig create mode 100644 drivers/dpll/Makefile create mode 100644 drivers/dpll/dpll_core.c create mode 100644 drivers/dpll/dpll_core.h create mode 100644 drivers/dpll/dpll_netlink.c create mode 100644 drivers/dpll/dpll_netlink.h create mode 100644 include/linux/dpll.h create mode 100644 include/uapi/linux/dpll.h