Re: [PATCH RFC v6 2/6] dpll: Add DPLL framework base functions

[Jiri Pirko <jiri@resnulli.us>]

Thu, May 04, 2023 at 04:16:43AM CEST, kuba@kernel.org wrote:
>On Wed, 3 May 2023 09:56:57 +0200 Jiri Pirko wrote:
>> >Yup, non-deterministic, just a cyclic ID allocated by the core starting
>> >from 1. Finding the right device / pin needs to be done via
>> >informational attributes not making assumptions about the ID.  
>> 
>> Okay.
>> 
>> When netdev will have pin ID in the RT netlink message (as it is done
>> in RFCv7), it is easy to get the pin/dpll for netdev. No problem there.
>> 
>> However, for non-SyncE usecase, how do you imagine scripts to work?
>> I mean, the script have to obtain dpll/pin ID by deterministic
>> module_name/clock_id/idx tuple.
>
>No scoped idx.

That means, no index defined by a driver if I undestand you correctly,
right?


>
>> There are 2 options to do that:
>> 1) dump all dplls/pins and do lookup in userspace
>> 2) get a dpll/pin according to given module_name/clock_id/idx tuple
>> 
>> The first approach is not very nice.
>> The currently pushed RFCv7 of the patchset does not support 2)
>> 
>> Now if we add support for 2), we basically use module_name/clock_id/idx
>> as a handle for "get cmd". My point is, why can't we use it for "set
>> cmd" as well and avoid the ID entirely?
>
>Sure, we don't _have_ to have an ID, but it seems go against normal
>data normalization rules. And I don't see any harm in it.
>
>But you're asking for per-device "idx" and that's a no-go for me,
>given already cited experience.
>
>The user space can look up the ID based on identifying information it
>has. IMO it's better to support multiple different intelligible elements

Do you mean fixed tuple or variable tuple?

CMD_GET_ID
  -> DPLL_A_MODULE_NAME
     DPLL_A_CLOCK_ID
  <- DPLL_A_ID

CMD_GET_PIN_ID
  -> DPLL_A_MODULE_NAME
     DPLL_A_CLOCK_ID
  <- DPLL_A_PIN_ID



>than single integer index into which drivers will start encoding all
>sort of info, using locally invented schemes.

There could be multiple DPLL and pin instances for a single
module/clock_id tuple we have to distinguish somehow. If the driver
can't pass "index" of DPLL or a pin, how we distinguish them?

Plus is is possible that 2 driver instances share the same dpll
instance, then to get the dpll pointer reference, they do:
INSTANCE A:
dpll_0 = dpll_device_get(clock_id, 0, THIS_MODULE);
dpll_1 = dpll_device_get(clock_id, 1, THIS_MODULE);

INSTANCE B:
dpll_0 = dpll_device_get(clock_id, 0, THIS_MODULE);
dpll_1 = dpll_device_get(clock_id, 1, THIS_MODULE);

My point is, event if we don't expose the index to the userspace,
we need to have it internally.