netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>
Cc: Vadim Fedorenko <vadim.fedorenko@linux.dev>,
	Jiri Pirko <jiri@resnulli.us>,
	Jonathan Lemon <jonathan.lemon@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>,
	"Olech, Milena" <milena.olech@intel.com>,
	"Michalik, Michal" <michal.michalik@intel.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, poros <poros@redhat.com>,
	mschmidt <mschmidt@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
	Bart Van Assche <bvanassche@acm.org>,
	"intel-wired-lan@lists.osuosl.org"
	<intel-wired-lan@lists.osuosl.org>, Jiri Pirko <jiri@nvidia.com>
Subject: Re: [PATCH net-next v4 2/9] dpll: spec: Add Netlink spec in YAML
Date: Thu, 17 Aug 2023 16:36:40 -0700	[thread overview]
Message-ID: <20230817163640.2ad33a4b@kernel.org> (raw)
In-Reply-To: <DM6PR11MB4657AD95547A14234941F9399B1AA@DM6PR11MB4657.namprd11.prod.outlook.com>

On Thu, 17 Aug 2023 18:40:00 +0000 Kubalewski, Arkadiusz wrote:
> >Why are all attributes in a single attr space? :(
> >More than half of them are prefixed with a pin- does it really
> >not scream to you that they belong to a different space?
> 
> I agree, but there is an issue with this, currently:
> 
> name: pin-parent-device
> subset-of: dpll
> attributes:
>   -
>     name: id
>     type: u32
>   -
>     name: pin-direction
>     type: u32
>   -
>     name: pin-prio
>     type: u32
>   -
>     name: pin-state
>     type: u32
> 
> Where "id" is a part of device space, rest attrs would be a pin space..
> Shall we have another argument for device id in a pin space?

Why would pin and device not have separate spaces?

When referring to a pin from a "device mostly" command you can
usually wrap the pin attributes in a nest, and vice versa.
But it may not be needed at all here? Let's look at the commands:

+    -
+      name: device-id-get
+        request:
+          attributes:
+            - module-name
+            - clock-id
+            - type
+        reply:
+          attributes:
+            - id

All attributes are in "device" space, no mixing.

+      name: device-get
+        request:
+          attributes:
+            - id
+        reply: &dev-attrs
+          attributes:
+            - id
+            - module-name
+            - mode
+            - mode-supported
+            - lock-status
+            - temp
+            - clock-id
+            - type

Again, no pin attributes, so pin can be separate?

+    -
+      name: device-set
+        request:
+          attributes:
+            - id

Herm, this one looks like it's missing attrs :S

+    -
+      name: pin-id-get
+        request:
+          attributes:
+            - module-name
+            - clock-id
+            - pin-board-label
+            - pin-panel-label
+            - pin-package-label
+            - pin-type
+        reply:
+          attributes:
+            - pin-id

Mostly pin stuff. I guess the module-name and clock-id attrs can be
copy/pasted between device and pin, or put them in a separate set
and add that set as an attr here. Copy paste is likely much simpler.

+    -
+      name: pin-get
+        request:
+          attributes:
+            - pin-id
+        reply: &pin-attrs
+          attributes:
+            - pin-id
+            - pin-board-label
+            - pin-panel-label
+            - pin-package-label
+            - pin-type
+            - pin-frequency
+            - pin-frequency-supported
+            - pin-dpll-caps
+            - pin-parent-device
+            - pin-parent-pin

All pin.

+    -
+      name: pin-set
+        request:
+          attributes:
+            - pin-id
+            - pin-frequency
+            - pin-direction
+            - pin-prio
+            - pin-state
+            - pin-parent-device
+            - pin-parent-pin

And all pin.

  reply	other threads:[~2023-08-17 23:36 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-11 20:03 [PATCH net-next v4 0/9] Create common DPLL configuration API Vadim Fedorenko
2023-08-11 20:03 ` [PATCH net-next v4 1/9] dpll: documentation on DPLL subsystem interface Vadim Fedorenko
2023-08-15  2:52   ` Jakub Kicinski
2023-08-11 20:03 ` [PATCH net-next v4 2/9] dpll: spec: Add Netlink spec in YAML Vadim Fedorenko
2023-08-15  2:43   ` Jakub Kicinski
2023-08-17 18:40     ` Kubalewski, Arkadiusz
2023-08-17 23:36       ` Jakub Kicinski [this message]
2023-08-18  7:23         ` Jiri Pirko
2023-08-21 10:15           ` Kubalewski, Arkadiusz
2023-08-22 16:54             ` Jakub Kicinski
2023-08-11 20:03 ` [PATCH net-next v4 3/9] dpll: core: Add DPLL framework base functions Vadim Fedorenko
2023-08-15  3:17   ` Jakub Kicinski
2023-08-15  6:00     ` Jiri Pirko
2023-08-15 18:20     ` Vadim Fedorenko
2023-08-15 18:28       ` Jakub Kicinski
2023-08-15 18:38         ` Vadim Fedorenko
2023-08-11 20:03 ` [PATCH net-next v4 4/9] dpll: netlink: " Vadim Fedorenko
2023-08-15  3:23   ` Jakub Kicinski
2023-08-15  3:24   ` Jakub Kicinski
2023-08-15 15:18     ` Vadim Fedorenko
2023-08-15 16:55       ` Jakub Kicinski
2023-08-15 18:25         ` Vadim Fedorenko
2023-08-11 20:03 ` [PATCH net-next v4 5/9] netdev: expose DPLL pin handle for netdevice Vadim Fedorenko
2023-08-11 20:03 ` [PATCH net-next v4 6/9] ice: add admin commands to access cgu configuration Vadim Fedorenko
2023-08-11 20:03 ` [PATCH net-next v4 7/9] ice: implement dpll interface to control cgu Vadim Fedorenko
2023-08-12  6:19   ` Jiri Pirko
2023-08-11 20:03 ` [PATCH net-next v4 8/9] ptp_ocp: implement DPLL ops Vadim Fedorenko
2023-08-11 20:03 ` [PATCH net-next v4 9/9] mlx5: Implement SyncE support using DPLL infrastructure Vadim Fedorenko
2023-08-12  6:22 ` [PATCH net-next v4 0/9] Create common DPLL configuration API Jiri Pirko
2023-08-12 11:20   ` Vadim Fedorenko
2023-08-15  2:45 ` Jakub Kicinski
2023-08-15 11:36   ` Vadim Fedorenko
2023-08-15 11:52     ` Jiri Pirko
2023-08-15 14:32       ` Vadim Fedorenko
2023-08-15 17:02       ` Jakub Kicinski
2023-08-18 10:15         ` Kubalewski, Arkadiusz
2023-08-18 21:03           ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230817163640.2ad33a4b@kernel.org \
    --to=kuba@kernel.org \
    --cc=arkadiusz.kubalewski@intel.com \
    --cc=bvanassche@acm.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jiri@nvidia.com \
    --cc=jiri@resnulli.us \
    --cc=jonathan.lemon@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=michal.michalik@intel.com \
    --cc=milena.olech@intel.com \
    --cc=mschmidt@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=poros@redhat.com \
    --cc=vadim.fedorenko@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).