Wed, Jul 02, 2025 at 04:51:47PM +0200, ivecera@xxxxxxxxxx wrote: >On 02. 07. 25 2:01 odp., Jiri Pirko wrote: >> Wed, Jul 02, 2025 at 01:43:38PM +0200, ivecera@xxxxxxxxxx wrote: >> > >> > On 02. 07. 25 12:31 odp., Jiri Pirko wrote: >> > > Sun, Jun 29, 2025 at 09:10:42PM +0200, ivecera@xxxxxxxxxx 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.
