linux-kernel - Re: [PATCH net-next v12 07/14] dpll: zl3073x: Add clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <bacab4b5-5c7f-4ece-9ca9-08723ec91aec@redhat.com>
Date: Thu, 3 Jul 2025 12:45:31 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Jiri Pirko <jiri@...nulli.us>, Krzysztof Kozlowski <krzk+dt@...nel.org>
Cc: netdev@...r.kernel.org, Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
 Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Prathosh Satish <Prathosh.Satish@...rochip.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
 Jason Gunthorpe <jgg@...pe.ca>, Shannon Nelson <shannon.nelson@....com>,
 Dave Jiang <dave.jiang@...el.com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 Michal Schmidt <mschmidt@...hat.com>, Petr Oros <poros@...hat.com>
Subject: Re: [PATCH net-next v12 07/14] dpll: zl3073x: Add clock_id field

On 03. 07. 25 12:09 odp., Jiri Pirko wrote:
> Wed, Jul 02, 2025 at 04:51:47PM +0200, ivecera@...hat.com wrote:
>> On 02. 07. 25 2:01 odp., Jiri Pirko wrote:
>>> Wed, Jul 02, 2025 at 01:43:38PM +0200, ivecera@...hat.com wrote:
>>>>
>>>> On 02. 07. 25 12:31 odp., Jiri Pirko wrote:
>>>>> Sun, Jun 29, 2025 at 09:10:42PM +0200, ivecera@...hat.com wrote:
>>>>>> Add .clock_id to zl3073x_dev structure that will be used by later
>>>>>> commits introducing DPLL feature. The clock ID is required for DPLL
>>>>>> device registration.
>>>>>>
>>>>>> To generate this ID, use chip ID read during device initialization.
>>>>>> In case where multiple zl3073x based chips are present, the chip ID
>>>>>> is shifted and lower bits are filled by an unique value - using
>>>>>> the I2C device address for I2C connections and the chip-select value
>>>>>> for SPI connections.
>>>>>
>>>>> You say that multiple chips may have the same chip ID? How is that
>>>>> possible? Isn't it supposed to be unique?
>>>>> I understand clock ID to be invariant regardless where you plug your
>>>>> device. When you construct it from i2c address, sounds wrong.
>>>>
>>>> The chip id is not like serial number but it is like device id under
>>>> PCI. So if you will have multiple chips with this chip id you have to
>>>> distinguish somehow between them, this is the reason why I2C address
>>>> is added into the final value.
>>>>
>>>> Anyway this device does not have any attribute that corresponds to
>>>> clock id (as per our previous discussion) and it will be better to NOT
>>>> require clock id from DPLL core side.
>>>
>>> Yes, better not to require it comparing to having it wrong.
>>
>> It looks that using clock_id==0 is safe from DPLL API point of view.
>> The problem is if you will have multiple zl3073x based chips because
>> the driver would call dpll_device_get(0 /* clock_id */, channel, module)
>>
>> For 1st chip (e.g. 2 channel) the driver will call:
>> dpll_device_get(0, 0, module);
>> dpll_device_get(0, 1, module);
>>
>> and for the second the same that is wrong. The clock_id would help to
>> distinguish between them.
>>
>> Wouldn't it be better to use a random number for clock_id from the
>> driver?
> 
> I take my suggestion to not require it back, does not make sense.
> 
> Clock id actually has a reason to exist from UAPI perspective. Checkout
> dpll_device_find_from_nlattr(). The user passes CLOCK_ID attr (among
> others) to obtain device by DPLL_CMD_DEVICE_ID_GET command. He expects
> to get a result back from kernel regardless where the device is plugged
> and across the reboots/rebinds.
> 
> Clock id should be properly filled with static and device specific
> value. If your chip can't be queried for it, I'm sure the embedded world
> has a solution for such cases. It's similar to MAC of a NIC device.

Yes, there are such cases and for such devices 'mac-address' property
can be specified in the device tree.

For our case I could extend the dpll device schema to include 'clock-id'
or 'dpll-clock-id' 64bit property to allow specify clock ID for the
devices that are unable to query this information from the hardware.

Krzysztof, WDYT about it?

Thanks,
Ivan